From 62ea7ef906c0880d22eb19a670559d804fa0504a Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 22 Sep 2026 03:42:56 -0700 Subject: [PATCH 1/8] feat: gate the dashboard version pill's update actions on a session that presented a second factor, carry session_mfa_verified on the web User model, pass the API error code into the permission-denied event and give admin_mfa_required its own two-factor dialog variant linking to Settings > Security instead of the Roles & access advice, and document the rule on the updates page --- docs/content/docs/development/updates.mdx | 1 + .../app/modals/PermissionDeniedModal.tsx | 42 +++++++++++++++---- web/src/components/layout/VersionPill.tsx | 32 ++++++++++---- web/src/hooks/usePermission.ts | 16 +++++++ web/src/lib/api/client/Request.ts | 2 +- web/src/lib/api/models/auth/User.ts | 3 ++ 6 files changed, 81 insertions(+), 15 deletions(-) diff --git a/docs/content/docs/development/updates.mdx b/docs/content/docs/development/updates.mdx index 65076cef5..8b0cb3e87 100644 --- a/docs/content/docs/development/updates.mdx +++ b/docs/content/docs/development/updates.mdx @@ -34,6 +34,7 @@ Every member of a self-hosted workspace sees the same version pill in the **dash Who can act on it follows platform admin access, not workspace roles: - **Members** see a badge. Its tooltip names the version and says to ask a platform admin. +- **Platform admins whose session has no second factor** get the same badge, and clicking it explains that administrative access needs two-factor authentication, with a link to Settings > Security. Turn on 2FA or add a passkey there and sign in again; the same rule gates the admin panel. - **Platform admins** click it and get the update dialog: the running and available versions with the release notes, the checkout and updater state, a "Check now" button, and **Update and restart**. That button leads to a confirmation pane that spells out what the update does (pull, rebuild and restart, sending pauses and resumes, migrations apply, the tab reconnects) before anything runs. While the update runs the dialog shows a progress bar, the step list with the live step highlighted, and the log behind a toggle. When the backend goes away for the restart the dialog says it is reconnecting and keeps polling; the pill in the header turns into a spinner so the job stays visible with the dialog closed, and a reload picks it back up. When the new backend answers, the dialog shows the result and reloads the dashboard after a short countdown, or, if the dialog was closed, a toast reports the new version and every list refreshes. diff --git a/web/src/components/app/modals/PermissionDeniedModal.tsx b/web/src/components/app/modals/PermissionDeniedModal.tsx index 123299ac1..6bbee4e01 100644 --- a/web/src/components/app/modals/PermissionDeniedModal.tsx +++ b/web/src/components/app/modals/PermissionDeniedModal.tsx @@ -5,21 +5,24 @@ import React from "react"; import { AnimatePresence, motion } from "framer-motion"; -import { LockIcon, SparklesIcon, XIcon } from "lucide-react"; +import { LockIcon, ShieldCheckIcon, SparklesIcon, XIcon } from "lucide-react"; import { Link } from "react-router-dom"; interface DeniedDetail { message?: string; + code?: string; } export default function PermissionDeniedModal() { const [open, setOpen] = React.useState(false); const [message, setMessage] = React.useState(""); + const [code, setCode] = React.useState(undefined); React.useEffect(() => { const handler = (e: Event) => { const detail = (e as CustomEvent).detail; setMessage(detail?.message?.trim() || "You don't have permission to do that."); + setCode(detail?.code); setOpen(true); }; window.addEventListener("permission-denied", handler); @@ -28,7 +31,11 @@ export default function PermissionDeniedModal() { // Plan/billing gates come back as 403 too, but they aren't a role problem — // label them as an upgrade prompt so the message and the call to action match. - const isPlan = /\b(plan|upgrade|trial|subscription|paid)\b/i.test(message); + // Admin routes refuse a session that never presented a second factor. That + // is fixed under Settings > Security by the person themselves, so the + // "ask an admin" advice would be wrong; link them there instead. + const isMFA = code === "admin_mfa_required"; + const isPlan = !isMFA && /\b(plan|upgrade|trial|subscription|paid)\b/i.test(message); const close = () => setOpen(false); return ( @@ -63,17 +70,29 @@ export default function PermissionDeniedModal() { className={`mx-auto mb-3 size-11 rounded-xl border flex items-center justify-center ${ isPlan ? "bg-violet-50 border-violet-200 text-violet-600" - : "bg-amber-50 border-amber-200 text-amber-600" + : isMFA + ? "bg-sky-50 border-sky-200 text-sky-600" + : "bg-amber-50 border-amber-200 text-amber-600" }`} > - {isPlan ? : } + {isPlan ? ( + + ) : isMFA ? ( + + ) : ( + + )}

- {isPlan ? "Upgrade required" : "You don't have permission"} + {isPlan + ? "Upgrade required" + : isMFA + ? "Two-factor authentication required" + : "You don't have permission"}

{message} - {!isPlan && ( + {!isPlan && !isMFA && ( <> {" "} Ask a workspace admin or the owner to grant you access from{" "} @@ -94,11 +113,20 @@ export default function PermissionDeniedModal() { View plans )} + {isMFA && ( + + Open security settings + + )} + ); + } if (!isAdmin) { return ( diff --git a/web/src/hooks/usePermission.ts b/web/src/hooks/usePermission.ts index 1410f4a98..107d39b31 100644 --- a/web/src/hooks/usePermission.ts +++ b/web/src/hooks/usePermission.ts @@ -62,6 +62,22 @@ export function showPermissionDenied(key: PermissionKey) { ); } +// An admin action from a session that never presented a second factor. The +// backend answers admin_mfa_required; raising it before the request gives the +// same dialog without a round trip that is known to fail. +export function showAdminMFARequired() { + if (typeof window === "undefined") return; + window.dispatchEvent( + new CustomEvent("permission-denied", { + detail: { + code: "admin_mfa_required", + message: + "Administrative access requires two-factor authentication. Turn on 2FA or add a passkey under Settings > Security, then sign in again.", + }, + }), + ); +} + export interface WriteGuard { /** Whether the current member may perform this write. */ allowed: boolean; diff --git a/web/src/lib/api/client/Request.ts b/web/src/lib/api/client/Request.ts index 7258a5736..fc4e808fa 100644 --- a/web/src/lib/api/client/Request.ts +++ b/web/src/lib/api/client/Request.ts @@ -194,7 +194,7 @@ export default async function Request(config: AuthRequestConfig): Promise if (method !== "GET" && method !== "HEAD") { window.dispatchEvent( new CustomEvent("permission-denied", { - detail: { message: appErr.message }, + detail: { message: appErr.message, code: appErr.code }, }), ); } diff --git a/web/src/lib/api/models/auth/User.ts b/web/src/lib/api/models/auth/User.ts index 55fdc7be2..8bb01d401 100644 --- a/web/src/lib/api/models/auth/User.ts +++ b/web/src/lib/api/models/auth/User.ts @@ -21,6 +21,9 @@ export default interface User { // permission; the dashboard only uses it to link to the admin panel. is_admin?: boolean; admin_permissions?: number; + // Whether this session presented a second factor. Admin routes refuse a + // session that did not, so admin actions in the dashboard gate on it too. + session_mfa_verified?: boolean; tags: Tag[]; categories: Category[]; From 4a6f0c17c7f16b60246838905deae6db61050e9b Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 22 Sep 2026 04:39:18 -0700 Subject: [PATCH 2/8] feat: advance each IMAP folder's sync cursor to the SELECT view its search ran against instead of the earlier LIST-STATUS, so Sent copies appended between the two are no longer skipped, and drop the message-map entry when NEW_EMAIL fails to publish so an unpublished message is re-offered instead of read as known (#645) --- internal/app/worker/wmail/admit.go | 10 +- internal/app/worker/wmail/imap_conn.go | 3 + internal/app/worker/wmail/sync_imap.go | 51 ++++++--- internal/app/worker/wmail/sync_imap_test.go | 114 ++++++++++++++++++++ internal/client/smtpimap/imap/client.go | 28 +++++ 5 files changed, 192 insertions(+), 14 deletions(-) diff --git a/internal/app/worker/wmail/admit.go b/internal/app/worker/wmail/admit.go index 375fefabc..339da35b1 100644 --- a/internal/app/worker/wmail/admit.go +++ b/internal/app/worker/wmail/admit.go @@ -181,11 +181,19 @@ func (w *WMail) storeNew(ctx context.Context, msg *models.EmailMessageData, data } // The consumer decodes NEW_EMAIL as JobEventNewEmail{user_id, message}. - return w.onEvent(models.JobEventTypeNewEmail, &models.JobEventNewEmail{ + err := w.onEvent(models.JobEventTypeNewEmail, &models.JobEventNewEmail{ UserID: w.UserID, Message: data, ReportOriginalMessageID: reportAbout, }) + if err != nil { + // The entry would mark a message that never reached the unibox as + // known, and every later pass would skip it; drop it so it is re-offered. + if derr := w.EmailMessageMapRepository.Del(ctx, w.UserID, w.ID, mapKey, data.ID); derr != nil { + log.Warn().Err(derr).Str("email_id", w.ID.String()).Msg("sync: map entry for an unpublished message not removed") + } + } + return err } // capBody bounds a stored body part at MaxEmailBodySize. IMAP already reads diff --git a/internal/app/worker/wmail/imap_conn.go b/internal/app/worker/wmail/imap_conn.go index e5fd8d4fa..49c00ce2c 100644 --- a/internal/app/worker/wmail/imap_conn.go +++ b/internal/app/worker/wmail/imap_conn.go @@ -26,6 +26,9 @@ type ImapConn interface { HasCondStore() bool ReleaseMailbox() SelectForSync(mailbox string) (uint32, *errx.MailError) + // SelectForSyncState selects like SelectForSync and reports the selected + // view's cursors, which an incremental pass advances the folder to. + SelectForSyncState(mailbox string) (imap.Selected, *errx.MailError) SearchChangedSince(modSeq uint64) ([]goimap.UID, *errx.MailError) SearchNewSince(uidNext uint32) ([]goimap.UID, *errx.MailError) // SearchAll is the folder's complete UID set, the presence side of the diff --git a/internal/app/worker/wmail/sync_imap.go b/internal/app/worker/wmail/sync_imap.go index b8e943f0c..4eae926f6 100644 --- a/internal/app/worker/wmail/sync_imap.go +++ b/internal/app/worker/wmail/sync_imap.go @@ -95,13 +95,15 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { changed := imapFolderChanged(befBox, box, condStore) fullyProcessed := true var touched map[string]struct{} + var view imap.Selected if changed && !stats.aborted { w.setWalking(box) - done, ids, err := w.imapIncremental(ctx, box, befBox, condStore, stats) + done, sel, ids, err := w.imapIncremental(ctx, box, befBox, condStore, stats) if err != nil { return err } fullyProcessed = done + view = sel touched = ids } else if changed { // The pass was aborted before this folder; hold its cursor too. @@ -115,6 +117,8 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { if !fullyProcessed { next.HighestModSeq = befBox.HighestModSeq next.UIDNext = befBox.UIDNext + } else if changed { + advanceToView(&next, view) } if err := w.mboxEvent(&next); err != nil { return nil @@ -213,16 +217,22 @@ func imapFolderChanged(before, now *models.Mailbox, condStore bool) bool { // imapIncremental stores what changed in one folder since the held cursor. // Known messages relay their flags unbudgeted; new ones are admitted newest // first. It reports whether every change was stored, which is what lets the -// folder's cursor advance, plus the Message-IDs it fetched so the drafts +// folder's cursor advance, the selected view the search ran against, which is +// where it advances to, and the Message-IDs it fetched so the drafts // reconciliation can tell a re-appended draft from an expunged one. -func (w *WMail) imapIncremental(ctx context.Context, box, before *models.Mailbox, condStore bool, stats *tickStats) (bool, map[string]struct{}, *errx.MailError) { +func (w *WMail) imapIncremental(ctx context.Context, box, before *models.Mailbox, condStore bool, stats *tickStats) (bool, imap.Selected, map[string]struct{}, *errx.MailError) { client := w.SmtpImapData.ImapClient - count, err := client.SelectForSync(box.Name) + view, err := client.SelectForSyncState(box.Name) if err != nil { - return false, nil, err + return false, view, nil, err } - if count == 0 { - return true, nil, nil + // The listing and this view name different generations, so the search + // would answer about UIDs the cursor does not; the next pass re-baselines. + if view.UIDValidity != 0 && view.UIDValidity != box.UIDValidity { + return false, view, nil, nil + } + if view.Count == 0 { + return true, view, nil, nil } var uids []goimap.UID if condStore { @@ -231,10 +241,10 @@ func (w *WMail) imapIncremental(ctx context.Context, box, before *models.Mailbox uids, err = client.SearchNewSince(before.UIDNext) } if err != nil { - return false, nil, err + return false, view, nil, err } if len(uids) == 0 { - return true, nil, nil + return true, view, nil, nil } // Newest first: when budget is short, the freshest mail lands first. sort.Slice(uids, func(i, j int) bool { return uids[i] > uids[j] }) @@ -250,11 +260,11 @@ func (w *WMail) imapIncremental(ctx context.Context, box, before *models.Mailbox hi := min(lo+config.ImapFetchBatchSize, len(uids)) fetched, err := client.FetchEnvelopes(ctx, uids[lo:hi]) if err != nil { - return false, touched, err + return false, view, touched, err } done, err := w.imapApply(ctx, fetched, false, stats) if err != nil { - return false, touched, err + return false, view, touched, err } for _, f := range fetched { if touched != nil { @@ -266,10 +276,25 @@ func (w *WMail) imapIncremental(ctx context.Context, box, before *models.Mailbox // unfrozen on a long backlog would deactivate itself walking mail it // cannot keep. Stop here; the held mod-sequence re-offers the rest. if !done || stats.aborted || ctx.Err() != nil { - return false, touched, nil + return false, view, touched, nil } } - return true, touched, nil + return true, view, touched, nil +} + +// advanceToView moves a fully walked folder's cursor to the view its search +// ran against. The listing's STATUS is taken before the SELECT, and a server +// whose selected view lags it (a session snapshot, an APPEND from the send +// path in between) would otherwise record a cursor past mail the search never +// returned, and that mail would never be synced. A view that reports no +// cursor keeps the listing's. +func advanceToView(next *models.Mailbox, view imap.Selected) { + if view.UIDNext != 0 { + next.UIDNext = view.UIDNext + } + if view.HighestModSeq != 0 { + next.HighestModSeq = view.HighestModSeq + } } // imapReconcileDrafts removes the platform's rows for drafts the server no diff --git a/internal/app/worker/wmail/sync_imap_test.go b/internal/app/worker/wmail/sync_imap_test.go index 6409ae46c..ffc3e6341 100644 --- a/internal/app/worker/wmail/sync_imap_test.go +++ b/internal/app/worker/wmail/sync_imap_test.go @@ -36,6 +36,9 @@ type fakeImapConn struct { // selectGen, when non-zero, is the UIDVALIDITY SELECT reports, which the // reconciliation compares against the one the listing gave it. selectGen uint32 + // view, when set, is the cursors SELECT reports in place of the listing's: + // a server whose selected view lags or leads its STATUS. + view *imap.Selected } func (c *fakeImapConn) Folders() ([]models.Mailbox, *errx.MailError) { return c.folders, nil } @@ -75,6 +78,19 @@ func (c *fakeImapConn) SelectForSync(string) (uint32, *errx.MailError) { return uint32(len(c.changed)), nil } +func (c *fakeImapConn) SelectForSyncState(name string) (imap.Selected, *errx.MailError) { + sel := imap.Selected{Count: uint32(len(c.changed))} + for _, f := range c.folders { + if f.Name == name { + sel.UIDValidity, sel.UIDNext, sel.HighestModSeq = f.UIDValidity, f.UIDNext, f.HighestModSeq + } + } + if c.view != nil { + sel.UIDNext, sel.HighestModSeq = c.view.UIDNext, c.view.HighestModSeq + } + return sel, nil +} + func (c *fakeImapConn) SelectForSyncGen(name string) (uint32, uint32, *errx.MailError) { gen := c.selectGen if gen == 0 { @@ -790,3 +806,101 @@ func TestImapSyncDoesNotReconcileExpungesOutsideDrafts(t *testing.T) { t.Errorf("asked the backend about INBOX %d times; only drafts are reconciled", ctx.calls) } } + +// The cursor is the selected view's, not the listing's. A server whose STATUS +// is ahead of the view the search ran on (an APPEND to Sent landing between +// the two) reported mail the search never returned, and advancing to the +// listing skipped that mail for good (#645). +func TestImapSyncAdvancesToTheSelectedViewNotTheListing(t *testing.T) { + conn := &fakeImapConn{ + folders: []models.Mailbox{{Name: "Sent", Attrs: []string{"\\Sent"}, UIDValidity: 7, HighestModSeq: 300}}, + changed: []goimap.UID{1, 2}, + view: &imap.Selected{HighestModSeq: 200}, + } + w, _ := newIMAPTestMail(conn, &fixedBudget{allow: 10}, + &models.Mailbox{Name: "Sent", Attrs: []string{"\\Sent"}, UIDValidity: 7, HighestModSeq: 100}) + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + if got := w.SmtpImapData.Mailboxes[0].HighestModSeq; got != 200 { + t.Fatalf("mod-sequence = %d, want the selected view's 200", got) + } + + // The view catches up; the folder still differs from the cursor, so the + // next pass walks it again and reaches the message the first one missed. + conn.view = nil + conn.changed = []goimap.UID{1, 2, 3} + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("second Sync: %v", err) + } + if conn.fetches != 2 { + t.Errorf("fetched %d batches over two passes, want 2", conn.fetches) + } + if got := w.SmtpImapData.Mailboxes[0].HighestModSeq; got != 300 { + t.Errorf("mod-sequence = %d, want 300 once the view caught up", got) + } +} + +func TestImapSyncAdvancesUIDNextToTheSelectedView(t *testing.T) { + conn := &fakeImapConn{ + noCondStore: true, + folders: []models.Mailbox{{Name: "Sent", Attrs: []string{"\\Sent"}, UIDValidity: 7, UIDNext: 104}}, + changed: []goimap.UID{101, 102}, + view: &imap.Selected{UIDNext: 103}, + } + w, _ := newIMAPTestMail(conn, &fixedBudget{allow: 10}, + &models.Mailbox{Name: "Sent", Attrs: []string{"\\Sent"}, UIDValidity: 7, UIDNext: 101}) + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + if got := w.SmtpImapData.Mailboxes[0].UIDNext; got != 103 { + t.Errorf("UIDNEXT cursor = %d, want the selected view's 103", got) + } +} + +// recordingMessageMap remembers what was added and removed. +type recordingMessageMap struct { + fakeMessageMap + added, removed []string +} + +func (m *recordingMessageMap) Add(_ context.Context, d repository.EmailMessageData) error { + m.added = append(m.added, d.MessageID) + return nil +} + +func (m *recordingMessageMap) Del(_ context.Context, _, _ uuid.UUID, messageID string, _ uuid.UUID) error { + m.removed = append(m.removed, messageID) + return nil +} + +// A message whose NEW_EMAIL never reached the bus must not stay in the map, +// or every later pass reads it as known and it never reaches the unibox. +func TestStoreNewDropsTheMapEntryWhenThePublishFails(t *testing.T) { + conn := &fakeImapConn{ + folders: []models.Mailbox{{Name: "Sent", Attrs: []string{"\\Sent"}, UIDValidity: 7, HighestModSeq: 300}}, + changed: []goimap.UID{1}, + } + w, _ := newIMAPTestMail(conn, &fixedBudget{allow: 10}, + &models.Mailbox{Name: "Sent", Attrs: []string{"\\Sent"}, UIDValidity: 7, HighestModSeq: 100}) + m := &recordingMessageMap{} + w.EmailMessageMapRepository = m + w.onEvent = func(kind models.JobEventType, _ any) error { + if kind == models.JobEventTypeNewEmail { + return fmt.Errorf("bus unavailable") + } + return nil + } + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + if len(m.added) != 1 || len(m.removed) != 1 || m.removed[0] != m.added[0] { + t.Fatalf("added %v, removed %v: the unpublished message must leave the map", m.added, m.removed) + } + if got := w.SmtpImapData.Mailboxes[0].HighestModSeq; got != 100 { + t.Errorf("mod-sequence = %d, want the held 100", got) + } +} diff --git a/internal/client/smtpimap/imap/client.go b/internal/client/smtpimap/imap/client.go index 7dae930f0..59eac37df 100644 --- a/internal/client/smtpimap/imap/client.go +++ b/internal/client/smtpimap/imap/client.go @@ -334,6 +334,34 @@ func (c *Client) SelectForSyncGen(mailbox string) (uint32, uint32, *errx.MailErr return data.NumMessages, data.UIDValidity, nil } +// Selected is the view a SELECT opened: the count and the cursors every +// SEARCH on it answers against. +type Selected struct { + Count uint32 + UIDValidity uint32 + UIDNext uint32 + HighestModSeq uint64 +} + +// SelectForSyncState selects exactly as SelectForSync does and reports the +// cursors of the selected view, which is what a folder's stored cursor must +// advance to: a STATUS taken earlier can be ahead of the view the SEARCH ran on. +func (c *Client) SelectForSyncState(mailbox string) (Selected, *errx.MailError) { + c.lifecycle.RLock() + defer c.lifecycle.RUnlock() + defer c.begin()() + data, err := c.selectMailbox(mailbox, &imap.SelectOptions{ReadOnly: true, CondStore: c.condStore.Load()}) + if err != nil { + return Selected{}, c.handleError(err) + } + return Selected{ + Count: data.NumMessages, + UIDValidity: data.UIDValidity, + UIDNext: uint32(data.UIDNext), + HighestModSeq: data.HighestModSeq, + }, nil +} + // ReleaseMailbox drops the selected mailbox. Dovecot answers LIST-STATUS for // the selected mailbox with the values it held at SELECT, so a loop that keeps // INBOX selected never sees another change land. Servers without UNSELECT keep From 9009c6ec246751bb75a7046b7051e6ac03def2c8 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 22 Sep 2026 04:49:26 -0700 Subject: [PATCH 3/8] feat: build inbound bounce and complaint reports before NEW_EMAIL but publish them only after it succeeds, so a message re-offered after a failed arrival publish does not apply its deliverability report twice --- internal/app/worker/wmail/admit.go | 25 +++++++++--- internal/app/worker/wmail/bounce.go | 42 ++++++++++----------- internal/app/worker/wmail/sync_imap_test.go | 33 ++++++++++++++++ 3 files changed, 71 insertions(+), 29 deletions(-) diff --git a/internal/app/worker/wmail/admit.go b/internal/app/worker/wmail/admit.go index 339da35b1..2f9765d88 100644 --- a/internal/app/worker/wmail/admit.go +++ b/internal/app/worker/wmail/admit.go @@ -173,11 +173,14 @@ func (w *WMail) storeNew(ctx context.Context, msg *models.EmailMessageData, data // Both still run on every message: a report is one or the other in // practice, and deciding that here by skipping the second would be this // function guessing at a MIME question the parsers already answer. - bounceAbout := w.maybeEmitBounce(msg) - complaintAbout := w.maybeEmitComplaint(msg) - reportAbout := bounceAbout - if reportAbout == "" { - reportAbout = complaintAbout + bounce := w.bounceReport(msg) + complaint := w.complaintReport(msg) + var reportAbout string + switch { + case bounce != nil: + reportAbout = bounce.OriginalMessageID + case complaint != nil: + reportAbout = complaint.OriginalMessageID } // The consumer decodes NEW_EMAIL as JobEventNewEmail{user_id, message}. @@ -192,8 +195,18 @@ func (w *WMail) storeNew(ctx context.Context, msg *models.EmailMessageData, data if derr := w.EmailMessageMapRepository.Del(ctx, w.UserID, w.ID, mapKey, data.ID); derr != nil { log.Warn().Err(derr).Str("email_id", w.ID.String()).Msg("sync: map entry for an unpublished message not removed") } + return err } - return err + + // Reports go out only once the arrival is published, so a message + // re-offered after a failed publish does not apply its report twice. + if bounce != nil { + _ = w.onEvent(models.JobEventTypeInboundBounce, bounce) + } + if complaint != nil { + _ = w.onEvent(models.JobEventTypeInboundComplaint, complaint) + } + return nil } // capBody bounds a stored body part at MaxEmailBodySize. IMAP already reads diff --git a/internal/app/worker/wmail/bounce.go b/internal/app/worker/wmail/bounce.go index 1df4a31a4..995bc7381 100644 --- a/internal/app/worker/wmail/bounce.go +++ b/internal/app/worker/wmail/bounce.go @@ -8,8 +8,8 @@ import ( "github.com/warmbly/warmbly/internal/pkg/dsn" ) -// maybeEmitBounce inspects a freshly synced inbound message and, when it is a -// permanent delivery-status notification for one of our sends, emits an +// bounceReport inspects a freshly synced inbound message and, when it is a +// permanent delivery-status notification for one of our sends, builds the // INBOUND_BOUNCE event so the consumer can suppress the recipient and record the // bounce against the campaign. This is where API-sent (Gmail/Graph) mail finally // gets bounce tracking: those sends succeed synchronously, so the only bounce @@ -20,19 +20,19 @@ import ( // Best-effort and permanent-only: a message that doesn't parse to a permanent // failure with a resolvable original id is silently ignored, so a transient // (4.x.x) bounce never suppresses a valid recipient. -// It returns the send the NDR is about, so the arrival event can carry it: the +// The event names the send the NDR is about, so the arrival event can carry it: the // report's own body is the only place that id appears and the consumer cannot // read a body, but it is what decides whether the report is about a campaign // send the customer should see or a warmup send they never made. -func (w *WMail) maybeEmitBounce(msg *models.EmailMessageData) string { +func (w *WMail) bounceReport(msg *models.EmailMessageData) *models.JobEventInboundBounce { from := strings.Join(msg.From, " ") if !dsn.Detect(from, msg.Subject, headerFlagValue(msg.Flags, "Content-Type")) { - return "" + return nil } report := dsn.Parse(msg.BodyPlain + "\n" + msg.BodyHTML) if !report.Permanent { - return "" + return nil } // Resolve the original outbound Message-ID: the DSN body's returned headers @@ -42,18 +42,16 @@ func (w *WMail) maybeEmitBounce(msg *models.EmailMessageData) string { originalID = strings.Trim(msg.InReplyTo[len(msg.InReplyTo)-1], "<>") } if originalID == "" { - return "" // nothing to resolve the campaign send against + return nil // nothing to resolve the campaign send against } - originalID = strings.Trim(originalID, "<>") - _ = w.onEvent(models.JobEventTypeInboundBounce, &models.JobEventInboundBounce{ + return &models.JobEventInboundBounce{ UserID: w.UserID, EmailID: w.ID, - OriginalMessageID: originalID, + OriginalMessageID: strings.Trim(originalID, "<>"), FailedRecipient: report.FailedRecipient, Reason: msg.Subject, - }) - return originalID + } } // headerFlagValue reads a "Header:value" pseudo-flag out of the flag slice (the @@ -68,7 +66,7 @@ func headerFlagValue(flags []string, name string) string { return "" } -// maybeEmitComplaint emits INBOUND_COMPLAINT when a synced message is an abuse +// complaintReport builds INBOUND_COMPLAINT when a synced message is an abuse // feedback report for one of our sends. A complaint never arrives // synchronously; it comes back as mail, long after the send succeeded. // @@ -76,26 +74,24 @@ func headerFlagValue(flags []string, name string) string { // Microsoft Graph returns one rendered body and no parts, so a report synced // through Graph carries only its human notice and is not detected; reading // those needs a separate MIME fetch, which is not built. -// Like maybeEmitBounce, it returns the send the report is about so the arrival -// event can carry it. -func (w *WMail) maybeEmitComplaint(msg *models.EmailMessageData) string { +// Like bounceReport, the event names the send the report is about so the +// arrival event can carry it. +func (w *WMail) complaintReport(msg *models.EmailMessageData) *models.JobEventInboundComplaint { from := strings.Join(msg.From, " ") if !arf.Detect(from, msg.Subject, headerFlagValue(msg.Flags, "Content-Type")) { - return "" + return nil } report := arf.Parse(msg.BodyPlain + "\n" + msg.BodyHTML) if !report.IsComplaint || report.OriginalMessageID == "" { - return "" + return nil } - originalID := strings.Trim(report.OriginalMessageID, "<>") - _ = w.onEvent(models.JobEventTypeInboundComplaint, &models.JobEventInboundComplaint{ + return &models.JobEventInboundComplaint{ UserID: w.UserID, EmailID: w.ID, - OriginalMessageID: originalID, + OriginalMessageID: strings.Trim(report.OriginalMessageID, "<>"), ComplainedRecipient: report.ComplainedRecipient, Provider: report.UserAgent, - }) - return originalID + } } diff --git a/internal/app/worker/wmail/sync_imap_test.go b/internal/app/worker/wmail/sync_imap_test.go index ffc3e6341..6babed3a1 100644 --- a/internal/app/worker/wmail/sync_imap_test.go +++ b/internal/app/worker/wmail/sync_imap_test.go @@ -904,3 +904,36 @@ func TestStoreNewDropsTheMapEntryWhenThePublishFails(t *testing.T) { t.Errorf("mod-sequence = %d, want the held 100", got) } } + +// A bounce is applied once: its report goes out only after the arrival is +// published, so a message re-offered after a failed publish does not +// suppress and count against its campaign twice. +func TestStoreNewEmitsReportsOnlyAfterTheArrivalIsPublished(t *testing.T) { + msg := &models.EmailMessageData{ + MessageID: "", + From: []string{"Mail Delivery System (MAILER-DAEMON@mail.example.com)"}, + Subject: "Undelivered Mail Returned to Sender", + BodyPlain: "Final-Recipient: rfc822; nobody@invalid.example.com\nAction: failed\nStatus: 5.1.1\n\nMessage-ID: \n", + } + for _, publishFails := range []bool{true, false} { + w, _ := newIMAPTestMail(&fakeImapConn{}, &fixedBudget{}, &models.Mailbox{}) + var kinds []models.JobEventType + w.onEvent = func(kind models.JobEventType, _ any) error { + kinds = append(kinds, kind) + if kind == models.JobEventTypeNewEmail && publishFails { + return fmt.Errorf("bus unavailable") + } + return nil + } + data := &models.EmailMessageStoreData{ID: uuid.New(), MessageID: msg.MessageID} + _ = w.storeNew(t.Context(), msg, data, msg.MessageID) + + want := []models.JobEventType{models.JobEventTypeNewEmail} + if !publishFails { + want = append(want, models.JobEventTypeInboundBounce) + } + if fmt.Sprint(kinds) != fmt.Sprint(want) { + t.Errorf("publishFails=%v: events %v, want %v", publishFails, kinds, want) + } + } +} From fb453921db1b6e6625ae1c45756e1b3cad04306b Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 22 Sep 2026 04:59:27 -0700 Subject: [PATCH 4/8] feat: keep a failed message-map rollback pending on the mailbox and retry it at the start of every IMAP, Gmail and Graph sync pass, skipping the pass until it succeeds so an unpublished arrival is never read as known --- internal/app/worker/wmail/admit.go | 21 ++++++++- internal/app/worker/wmail/sync_google.go | 3 ++ internal/app/worker/wmail/sync_graph.go | 3 ++ internal/app/worker/wmail/sync_imap.go | 3 ++ internal/app/worker/wmail/sync_imap_test.go | 52 +++++++++++++++++++++ internal/app/worker/wmail/wmail.go | 3 ++ 6 files changed, 84 insertions(+), 1 deletion(-) diff --git a/internal/app/worker/wmail/admit.go b/internal/app/worker/wmail/admit.go index 2f9765d88..b1cf25062 100644 --- a/internal/app/worker/wmail/admit.go +++ b/internal/app/worker/wmail/admit.go @@ -4,6 +4,7 @@ import ( "context" "time" + "github.com/google/uuid" "github.com/rs/zerolog/log" "github.com/warmbly/warmbly/internal/config" "github.com/warmbly/warmbly/internal/errx" @@ -193,7 +194,11 @@ func (w *WMail) storeNew(ctx context.Context, msg *models.EmailMessageData, data // The entry would mark a message that never reached the unibox as // known, and every later pass would skip it; drop it so it is re-offered. if derr := w.EmailMessageMapRepository.Del(ctx, w.UserID, w.ID, mapKey, data.ID); derr != nil { - log.Warn().Err(derr).Str("email_id", w.ID.String()).Msg("sync: map entry for an unpublished message not removed") + log.Warn().Err(derr).Str("email_id", w.ID.String()).Msg("sync: map entry for an unpublished message not removed; retried next pass") + if w.unmapPending == nil { + w.unmapPending = map[string]uuid.UUID{} + } + w.unmapPending[mapKey] = data.ID } return err } @@ -209,6 +214,20 @@ func (w *WMail) storeNew(ctx context.Context, msg *models.EmailMessageData, data return nil } +// retryUnmap removes the map entries a failed publish could not, and reports +// whether none is left. A pass must not run while one is: it would read that +// message as known and move its cursor past it. +func (w *WMail) retryUnmap(ctx context.Context) bool { + for key, id := range w.unmapPending { + if err := w.EmailMessageMapRepository.Del(ctx, w.UserID, w.ID, key, id); err != nil { + log.Warn().Err(err).Str("email_id", w.ID.String()).Msg("sync: map entry for an unpublished message still not removed; pass skipped") + return false + } + delete(w.unmapPending, key) + } + return true +} + // capBody bounds a stored body part at MaxEmailBodySize. IMAP already reads // at most that much off the wire; Gmail and Graph hand over whole bodies, so // without this an oversized message stored unbounded on those providers. diff --git a/internal/app/worker/wmail/sync_google.go b/internal/app/worker/wmail/sync_google.go index 075cb9bd1..08f3ef7d8 100644 --- a/internal/app/worker/wmail/sync_google.go +++ b/internal/app/worker/wmail/sync_google.go @@ -22,6 +22,9 @@ func (w *WMail) SyncGoogle(ctx context.Context) *errx.MailError { w.beginTick() stats := &tickStats{} w.googleTick = stats + if !w.retryUnmap(ctx) { + return nil + } newHistoryID, err := w.GoogleData.Client.FetchHistory(ctx, w.GoogleData.LastHistoryID) if newHistoryID != 0 && newHistoryID != w.GoogleData.LastHistoryID { diff --git a/internal/app/worker/wmail/sync_graph.go b/internal/app/worker/wmail/sync_graph.go index 5514232a8..d34558db7 100644 --- a/internal/app/worker/wmail/sync_graph.go +++ b/internal/app/worker/wmail/sync_graph.go @@ -25,6 +25,9 @@ func (w *WMail) SyncGraph(ctx context.Context) *errx.MailError { w.beginTick() stats := &tickStats{} w.graphTick = stats + if !w.retryUnmap(ctx) { + return nil + } if err := w.GraphData.Client.Sync(ctx); err != nil { var mailErr *errx.MailError diff --git a/internal/app/worker/wmail/sync_imap.go b/internal/app/worker/wmail/sync_imap.go index 4eae926f6..20b90bd6e 100644 --- a/internal/app/worker/wmail/sync_imap.go +++ b/internal/app/worker/wmail/sync_imap.go @@ -31,6 +31,9 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { } w.beginTick() stats := &tickStats{} + if !w.retryUnmap(ctx) { + return nil + } client := w.SmtpImapData.ImapClient // A mailbox left selected by the previous pass freezes LIST-STATUS on this diff --git a/internal/app/worker/wmail/sync_imap_test.go b/internal/app/worker/wmail/sync_imap_test.go index 6babed3a1..89a1c76ae 100644 --- a/internal/app/worker/wmail/sync_imap_test.go +++ b/internal/app/worker/wmail/sync_imap_test.go @@ -864,6 +864,8 @@ func TestImapSyncAdvancesUIDNextToTheSelectedView(t *testing.T) { type recordingMessageMap struct { fakeMessageMap added, removed []string + // failDel is how many removals fail before they start succeeding. + failDel int } func (m *recordingMessageMap) Add(_ context.Context, d repository.EmailMessageData) error { @@ -872,6 +874,10 @@ func (m *recordingMessageMap) Add(_ context.Context, d repository.EmailMessageDa } func (m *recordingMessageMap) Del(_ context.Context, _, _ uuid.UUID, messageID string, _ uuid.UUID) error { + if m.failDel > 0 { + m.failDel-- + return fmt.Errorf("backend unavailable") + } m.removed = append(m.removed, messageID) return nil } @@ -937,3 +943,49 @@ func TestStoreNewEmitsReportsOnlyAfterTheArrivalIsPublished(t *testing.T) { } } } + +// A removal that fails too is retried before the next pass looks at anything, +// and the pass waits for it, so the message is never read as known. +func TestAFailedUnmapIsRetriedBeforeThePassRuns(t *testing.T) { + conn := &fakeImapConn{ + folders: []models.Mailbox{{Name: "Sent", Attrs: []string{"\\Sent"}, UIDValidity: 7, HighestModSeq: 300}}, + changed: []goimap.UID{1}, + } + w, _ := newIMAPTestMail(conn, &fixedBudget{allow: 10}, + &models.Mailbox{Name: "Sent", Attrs: []string{"\\Sent"}, UIDValidity: 7, HighestModSeq: 100}) + m := &recordingMessageMap{failDel: 2} + w.EmailMessageMapRepository = m + publishFails := true + arrivals := 0 + w.onEvent = func(kind models.JobEventType, _ any) error { + if kind == models.JobEventTypeNewEmail { + arrivals++ + if publishFails { + return fmt.Errorf("bus unavailable") + } + } + return nil + } + + // Publish fails and so does the rollback; the next pass cannot remove it + // either, so it runs nothing. + for range 2 { + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + } + if arrivals != 1 || conn.fetches != 1 { + t.Fatalf("arrivals %d, fetches %d: a pass ran with the entry still mapped", arrivals, conn.fetches) + } + + publishFails = false + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + if len(m.removed) != 1 || arrivals != 2 { + t.Errorf("removed %v, arrivals %d: the message was not re-offered once the entry was gone", m.removed, arrivals) + } + if got := w.SmtpImapData.Mailboxes[0].HighestModSeq; got != 300 { + t.Errorf("mod-sequence = %d, want 300", got) + } +} diff --git a/internal/app/worker/wmail/wmail.go b/internal/app/worker/wmail/wmail.go index c1d3c9212..85ea745c5 100644 --- a/internal/app/worker/wmail/wmail.go +++ b/internal/app/worker/wmail/wmail.go @@ -98,6 +98,9 @@ type WMail struct { // flagScan is the previous flag snapshot per folder name, used only on // IMAP servers without CONDSTORE, which cannot say what changed. flagScan map[string]*folderFlagScan + // unmapPending holds map entries for unpublished arrivals whose removal + // failed, keyed by map key; every pass retries them before it looks. + unmapPending map[string]uuid.UUID // transportFailures counts consecutive passes that could not reach the // mail server, which paces the retry and keeps one outage to one warning. transportFailures int From 5a967cc3cece76ea353bb7dbcb9e7c742ee371ea Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 22 Sep 2026 05:15:49 -0700 Subject: [PATCH 5/8] feat: let an IMAP mailbox exclude folders from sync (email_accounts.sync_skip_folders, migration 000196) so a folder another tool fills never reaches the unified inbox: the worker drops skipped folders and their subfolders before the walk, retires an already-synced one with its stored mail, and removes mail that later moves into one only when its Message-ID is found there; PUT /emails/:id/sync and the drawer's Sync card set the list, GET reports it with the server's folder list, warmbly mailbox skip-folders mirrors it, with docs, OpenAPI and error-code invalid_sync_folder --- cmd/backend/main.go | 3 + cmd/cli/specs.go | 18 +- cmd/warmblyctl/api_resources.go | 3 +- docs/content/docs/api/endpoints.mdx | 3 + docs/content/docs/api/error-codes.mdx | 1 + docs/content/docs/guides/mailboxes.mdx | 10 + docs/content/docs/guides/unibox.mdx | 2 +- docs/public/openapi.json | 168 ++++++++++++++- internal/api/handler/email_sync.go | 58 ++++- internal/api/routes.go | 1 + internal/app/consumer/event_mailbox_delete.go | 14 ++ internal/app/consumer/event_remove_email.go | 4 +- .../app/consumer/warmup_retention_test.go | 15 ++ internal/app/email/loader.go | 5 + internal/app/email/loader_status_test.go | 5 + internal/app/email/reauth_test.go | 5 + internal/app/email/service.go | 86 +++++++- internal/app/email/worker_removal_test.go | 5 + internal/app/worker/wmail/sync_imap.go | 142 +++++++++++++ .../app/worker/wmail/sync_imap_skip_test.go | 199 ++++++++++++++++++ internal/app/worker/wmail/sync_imap_test.go | 4 + internal/app/worker/wmail/wmail.go | 4 + internal/client/smtpimap/imap/folders.go | 86 ++++++++ .../client/smtpimap/imap/folders_skip_test.go | 80 +++++++ internal/config/constants.go | 2 + .../000196_email_sync_skip_folders.down.sql | 2 + .../000196_email_sync_skip_folders.up.sql | 14 ++ internal/models/event_w_emails.go | 4 + internal/models/event_w_mailbox.go | 3 + internal/models/mailbox.go | 5 + internal/models/sync.go | 19 ++ internal/repository/pg_email.go | 47 +++++ internal/repository/pg_unibox.go | 17 ++ skills/warmbly-api/SKILL.md | 2 +- skills/warmbly-cli/SKILL.md | 2 +- .../components/app/emails/InboxDetails.tsx | 2 +- .../components/app/emails/SyncStatusCard.tsx | 112 +++++++++- .../lib/api/client/app/emails/updateSync.ts | 12 ++ .../app/emails/useUpdateSyncSkipFolders.ts | 19 ++ .../lib/api/models/app/emails/SyncState.ts | 12 ++ 40 files changed, 1178 insertions(+), 17 deletions(-) create mode 100644 internal/app/worker/wmail/sync_imap_skip_test.go create mode 100644 internal/client/smtpimap/imap/folders_skip_test.go create mode 100644 internal/infrastructure/db/migrations/000196_email_sync_skip_folders.down.sql create mode 100644 internal/infrastructure/db/migrations/000196_email_sync_skip_folders.up.sql create mode 100644 web/src/lib/api/client/app/emails/updateSync.ts create mode 100644 web/src/lib/api/hooks/app/emails/useUpdateSyncSkipFolders.ts diff --git a/cmd/backend/main.go b/cmd/backend/main.go index 0c6fed2f7..343b1ece7 100644 --- a/cmd/backend/main.go +++ b/cmd/backend/main.go @@ -1185,6 +1185,9 @@ func main() { emailSyncStateRepository = repository.NewEmailSyncStateRepository(primaryDB) emailService.WireSyncState(emailSyncStateRepository) emailService.WireMailboxes(repository.NewMailboxRepository(primaryDB)) + // A folder excluded from sync has its already-stored mail dropped at + // the moment of the change, not a pass later. + emailService.WireUnibox(repository.NewUniboxRepository(primaryDB)) if instanceSettings != nil { emailService.WireSyncBudget(instanceSettings) } diff --git a/cmd/cli/specs.go b/cmd/cli/specs.go index 587ed3b9f..60bd3be22 100644 --- a/cmd/cli/specs.go +++ b/cmd/cli/specs.go @@ -661,10 +661,26 @@ so a removed alias stops being used instead of failing every send.`, Success: "Sending identity refreshed.", }, { - Name: "sync", Short: "The mailbox's sync state and backfill progress", + Name: "sync", Short: "The mailbox's sync state, backfill progress and skipped folders", Method: http.MethodGet, Path: "/emails/{id}/sync", Args: []argSpec{{Name: "id", Help: "The mailbox's id"}}, }, + { + Name: "skip-folders", Short: "Choose the folders an IMAP mailbox's sync leaves alone", + Long: `Replaces the list of folders the sync never opens, named as the mail +server lists them (see "mailbox sync" for the names). Each also covers its +subfolders. Mail already imported from a folder is removed from Warmbly +when it is skipped; the mail itself stays in the mailbox. Inbox, sent, +drafts, spam, trash and archive cannot be skipped. To sync everything +again, send an empty list with --input.`, + Example: " $ warmbly mailbox skip-folders MAILBOX_ID --folder Warmer\n $ warmbly mailbox skip-folders MAILBOX_ID --folder Warmer --folder \"Clients/Acme\"\n $ warmbly mailbox skip-folders MAILBOX_ID --input '{\"skip_folders\": []}'", + Method: http.MethodPut, Path: "/emails/{id}/sync", Body: bodyRequired, + Args: []argSpec{{Name: "id", Help: "The mailbox's id"}}, + Flag: []flagSpec{ + {Name: "folder", Help: "A folder to skip, as the server lists it (repeatable)", Kind: flagStrings, Key: "skip_folders"}, + }, + Success: "Skipped folders updated.", + }, { Name: "behavior", Short: "The mailbox's human-sending ranges", Method: http.MethodGet, Path: "/emails/{id}/behavior", diff --git a/cmd/warmblyctl/api_resources.go b/cmd/warmblyctl/api_resources.go index 07cdbacb5..e36d9c5ff 100644 --- a/cmd/warmblyctl/api_resources.go +++ b/cmd/warmblyctl/api_resources.go @@ -100,7 +100,8 @@ var apiSpecs = []apiSpec{ {name: "mailbox update", summary: "Update mailbox settings (limits, tags, timezone, signature)", method: "PATCH", path: "/emails/{id}", body: bodyRequired}, {name: "mailbox delete", summary: "Disconnect a mailbox", method: "DELETE", path: "/emails/{id}"}, {name: "mailbox auth-check", summary: "Check the mailbox's SPF, DKIM and DMARC", method: "GET", path: "/emails/{id}/auth-check"}, - {name: "mailbox sync", summary: "The mailbox's sync state and backfill progress", method: "GET", path: "/emails/{id}/sync"}, + {name: "mailbox sync", summary: "The mailbox's sync state, backfill progress and skipped folders", method: "GET", path: "/emails/{id}/sync"}, + {name: "mailbox skip-folders", summary: "Replace the folders an IMAP mailbox's sync leaves alone (body: {\"skip_folders\": [...]})", method: "PUT", path: "/emails/{id}/sync", body: bodyRequired}, {name: "mailbox identity", summary: "The addresses this mailbox may send as (Gmail only)", method: "GET", path: "/emails/{id}/identity"}, {name: "mailbox refresh-identity", summary: "Re-read the send-as addresses from the provider, optionally importing its signature", method: "POST", path: "/emails/{id}/identity/refresh", body: bodyOptional}, {name: "mailbox behavior", summary: "The mailbox's human-sending ranges", method: "GET", path: "/emails/{id}/behavior"}, diff --git a/docs/content/docs/api/endpoints.mdx b/docs/content/docs/api/endpoints.mdx index 99cd58a5d..e91399101 100644 --- a/docs/content/docs/api/endpoints.mdx +++ b/docs/content/docs/api/endpoints.mdx @@ -29,6 +29,7 @@ All paths below are relative to the versioned base URL `https://api.warmbly.com/ | POST | `/emails/:id/track/verify` | `WRITE_EMAILS` | | PATCH | `/emails/:id/direct-tracking` | `WRITE_EMAILS` | | GET | `/emails/:id/sync` | `READ_EMAILS` | +| PUT | `/emails/:id/sync` | `WRITE_EMAILS` | | GET | `/emails/:id/identity` | `READ_EMAILS` | | POST | `/emails/:id/identity/refresh` | `WRITE_EMAILS` | | GET | `/emails/:id/auth-check` | `READ_EMAILS` | @@ -235,6 +236,8 @@ The `/unibox/drafts` endpoints hold autosaved compose drafts, scoped to the call `GET /emails/:id/identity` reports which addresses the mailbox's provider will let it send as, which one it uses, and whether its signature was imported or written here; it contacts no provider. `POST /emails/:id/identity/refresh` re-reads that list from the provider and stores it, and with `import_signature` also replaces the stored signature with the one configured at the provider. Storing the list is what a `send_as_email` choice is validated against, so the refresh needs `WRITE_EMAILS` rather than `READ_EMAILS`; it takes no `Idempotency-Key` because it writes exactly what the provider currently says. Gmail only. See [sending identity](/guides/mailboxes/#sending-identity). +`GET /emails/:id/sync` reports, alongside the import state and budget, `skip_folders` (the folders the mailbox's sync leaves alone) and `folders` (what the worker last listed on the server, each with its `name` and the canonical `folder` it files under, INBOX first; empty for Gmail and Outlook). `PUT /emails/:id/sync` takes `{"skip_folders": [...]}` and replaces the list: names as the server lists them, matched without regard to case, each covering its subfolders. Mail already stored from a newly skipped folder is removed, the mailbox is re-shipped to its worker so the change applies on the next pass, and the response is the list as stored. It refuses `INBOX` and any sent, drafts, spam, trash or archive folder with `400 invalid_sync_folder`, as it does a list of more than `50` names or a mailbox that is not IMAP. The body is the desired state, so it takes no `Idempotency-Key`. See [folders you do not want synced](/guides/mailboxes/#folders-you-do-not-want-synced). + `PATCH /emails/:id` accepts `save_to_sent` (boolean) on SMTP/IMAP mailboxes: when true, which is the default, the worker files a copy of each outbound message in the mailbox's Sent folder. It has no effect on Gmail and Outlook mailboxes, whose APIs file their own copy. See [keeping a copy of sent mail](/guides/mailboxes/#keeping-a-copy-of-sent-mail). `DELETE /emails/:id` releases the mailbox on Warmbly Cloud before removing the local mailbox. An enrolled mailbox's stored credential comes out of the pool, and a cloud-managed mirror's claim is released so the mailbox returns to the cloud workspace and can be adopted again. If the cloud cannot confirm the release, the delete returns `409 mailbox_cloud_unenroll_failed` and keeps the mailbox record so the request can be retried safely. Warmbly attempts to restore the mailbox onto its worker immediately, and the worker reconciler may restore it later if that attempt fails. See [mailboxes](/guides/mailboxes/). diff --git a/docs/content/docs/api/error-codes.mdx b/docs/content/docs/api/error-codes.mdx index f76a48293..09c2097dc 100644 --- a/docs/content/docs/api/error-codes.mdx +++ b/docs/content/docs/api/error-codes.mdx @@ -94,6 +94,7 @@ Returned when the request cannot be processed due to invalid syntax. | `invalid_filter` | A task filter carried an id that is not one: `assigned_to`, `contact_id` and `deal_id` name records, and are matched against id columns. Sent by `POST /crm/tasks/search`, `POST /crm/tasks/summary`, and the `filters` of a `"all": true` bulk selection | | `invalid_setting` | `PATCH /outreach/settings` (or a campaign's advanced settings) carried a value outside the documented vocabulary, for example a `reply_intent.crm_task_intents` entry that is not a reply intent | | `invalid_slug` | `PATCH /organizations/current` was given a `slug` that is not 2 to 80 lowercase letters, numbers or dashes starting and ending with a letter or number | +| `invalid_sync_folder` | `PUT /emails/:id/sync` was given a folder the sync always follows (`INBOX`, or a sent, drafts, spam, trash or archive folder by attribute or name), a name that is empty after trimming, longer than 255 characters or carrying a control character, more than 50 names, or a mailbox that is not IMAP. The `message` names the entry refused | | `no_organization` | The request needs a workspace and the caller has none selected. Every entitlement, limit and suppression rule is scoped to a workspace, so a write that would run unscoped is refused rather than run without those checks. API keys always carry their workspace; a dashboard session picks one at sign-in, so this normally means the session predates the workspace being chosen. Select a workspace and retry | #### Password refusals diff --git a/docs/content/docs/guides/mailboxes.mdx b/docs/content/docs/guides/mailboxes.mdx index a32192cc0..030e26b76 100644 --- a/docs/content/docs/guides/mailboxes.mdx +++ b/docs/content/docs/guides/mailboxes.mdx @@ -183,6 +183,16 @@ Nested folders are followed as well, so mail in a subfolder of the inbox or unde On IMAP, folders are identified by the standard attributes a server publishes, and by name when it publishes none. The common names are recognized in a dozen languages, so a mailbox whose Sent folder is called "Gesendete Elemente" or "Éléments envoyés" still files sent mail as sent rather than as inbox. +### Folders you do not want synced + +A folder the sync does not recognize files as inbox, so its mail shows up in the [unified inbox](/guides/unibox/). That is right for a folder you sort real mail into and wrong for one another tool fills: a second warmup service running out of its own folder, for example, puts machine mail in your inbox, spends the mailbox's sync budget on it, and has it classified like a reply. + +On an IMAP mailbox, the drawer's **Sync** card lists the folders the worker has seen under **Folders not synced**. Tick one and the sync stops opening it on its next pass, within a minute; mail already imported from it is removed from Warmbly, and mail that later moves into it from a synced folder is removed too. The mail itself is never touched at the provider. A folder that is not listed yet, because the mailbox has not synced or the folder was just created, can be typed in by name, exactly as your mail server lists it (`Warmer`, or `INBOX/Warmer` on a server that nests everything under the inbox). Case does not matter, and a skipped folder covers its subfolders. + +The inbox, sent, drafts, spam, trash and archive folders cannot be skipped: a sync without them is a broken mailbox, not a quieter one. Un-ticking a folder brings it back the way a new folder arrives, syncing what lands in it from then on; the mail that accumulated while it was skipped is not imported. Up to `50` folders can be skipped per mailbox. Gmail and Outlook mailboxes have no such list. + +The same setting is `PUT /emails/:id/sync` in the [API](/api/endpoints/) and `warmbly mailbox skip-folders` in the CLI. + Some IMAP servers, including Outlook.com, Microsoft 365 over IMAP and Yahoo, cannot tell a client what changed since it last looked. New mail from those servers still arrives within a minute. Reading or flagging a message in another mail client shows up in Warmbly within about ten minutes rather than immediately. Nothing is lost either way, and Gmail, Outlook over OAuth, Fastmail and most self-hosted servers are immediate. diff --git a/docs/content/docs/guides/unibox.mdx b/docs/content/docs/guides/unibox.mdx index 09e6a7864..2175a474a 100644 --- a/docs/content/docs/guides/unibox.mdx +++ b/docs/content/docs/guides/unibox.mdx @@ -44,7 +44,7 @@ The rail is two short groups and then your mailboxes. The first group is where y | Folder | Group | Shows | | --- | --- | --- | -| Inbox | First | Inbound mail, plus anything filed in a custom folder at the provider | +| Inbox | First | Inbound mail, plus anything filed in a custom folder at the provider, unless that folder is [excluded from sync](/guides/mailboxes/#folders-you-do-not-want-synced) | | Drafts | Second | Messages sitting in a mailbox's drafts folder | | Sent | Second | Outbound mail, campaign and manual | | Archive | Second | Archived (Gmail's All Mail, the Archive folder elsewhere) | diff --git a/docs/public/openapi.json b/docs/public/openapi.json index d56318736..aa6ec10cd 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -2810,7 +2810,7 @@ "get": { "operationId": "mailboxes_sync_state", "summary": "Get sync state", - "description": "Where the mailbox's initial import stands, whether fair use is holding new mail, and the budget the mailbox syncs under. `state` is null until the worker has reported once.", + "description": "Where the mailbox's initial import stands, whether fair use is holding new mail, the budget the mailbox syncs under, the folders its sync leaves alone, and the folders the worker has seen on the server. `state` is null until the worker has reported once.", "tags": [ "mailboxes" ], @@ -2893,6 +2893,103 @@ } } } + }, + "put": { + "operationId": "mailboxes_sync_update", + "summary": "Set skipped folders", + "description": "Replaces the folders an IMAP mailbox's sync leaves alone. Names are matched as the mail server lists them, without regard to case, and each covers its subfolders. Mail already stored from a newly skipped folder is removed from Warmbly (never from the mailbox), and the mailbox is re-shipped to its worker so the change applies on the next pass. The body is the desired state, so the call is naturally idempotent. INBOX and the sent, drafts, spam, trash and archive folders cannot be skipped.", + "tags": [ + "mailboxes" + ], + "security": [ + { + "bearerAuth": [] + } + ], + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "description": "The mailbox id.", + "schema": { + "type": "string", + "format": "uuid" + } + } + ], + "responses": { + "200": { + "description": "The skip list as stored.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/MailboxSyncSettings" + } + } + } + }, + "400": { + "description": "Invalid id or body, or `invalid_sync_folder`: a folder the sync always follows, a malformed or over-long name, more than 50 names, or a mailbox that is not IMAP.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "401": { + "description": "Missing or invalid credentials.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Insufficient scope, permission, or mailbox not allowed for this key.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "404": { + "description": "Mailbox not found.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "429": { + "description": "Rate limited.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + } + }, + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/MailboxSyncSettings" + } + } + } + } } }, "/emails/{id}/identity": { @@ -21808,6 +21905,13 @@ "org_daily_messages": { "type": "integer", "description": "New plus imported messages stored across the organization per UTC day." + }, + "skip_folders": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The same skip list the mailbox carries to its worker; present when non-empty." } }, "required": [ @@ -21894,11 +21998,71 @@ }, "policy": { "$ref": "#/components/schemas/MailboxSyncPolicy" + }, + "skip_folders": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The folders the mailbox's sync leaves alone, as the server lists them. Replaced with `PUT /emails/{id}/sync`." + }, + "folders": { + "type": "array", + "items": { + "$ref": "#/components/schemas/MailboxSyncFolder" + }, + "description": "What the worker last listed on the server, INBOX first. Empty for Gmail and Outlook mailboxes." } }, "required": [ "state", - "policy" + "policy", + "skip_folders", + "folders" + ] + }, + "MailboxSyncFolder": { + "type": "object", + "description": "One folder the sync has seen on the server.", + "properties": { + "name": { + "type": "string", + "description": "The folder's name as the server lists it: the value to use in `skip_folders`." + }, + "folder": { + "type": "string", + "enum": [ + "inbox", + "sent", + "drafts", + "spam", + "trash", + "archive" + ], + "description": "The canonical folder it files under. Only `inbox` folders other than INBOX itself can be skipped." + } + }, + "required": [ + "name", + "folder" + ] + }, + "MailboxSyncSettings": { + "type": "object", + "description": "The folders an IMAP mailbox's sync leaves alone.", + "properties": { + "skip_folders": { + "type": "array", + "items": { + "type": "string", + "maxLength": 255 + }, + "maxItems": 50, + "description": "Folder names as the server lists them. Case does not matter; each covers its subfolders. An empty list syncs every folder." + } + }, + "required": [ + "skip_folders" ] }, "MailboxVerifyRequest": { diff --git a/internal/api/handler/email_sync.go b/internal/api/handler/email_sync.go index ec1895e77..d42e1f0b6 100644 --- a/internal/api/handler/email_sync.go +++ b/internal/api/handler/email_sync.go @@ -4,20 +4,30 @@ import ( "net/http" "github.com/gin-gonic/gin" + "github.com/google/uuid" "github.com/warmbly/warmbly/internal/api/middleware" "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" ) // emailSyncResponse is GET /emails/:id/sync: where the mailbox's import -// stands, whether fair use is holding it, and the budget it runs under. +// stands, whether fair use is holding it, the budget it runs under, the +// folders the owner excluded, and the folders the sync has seen so a client +// can offer them by name. type emailSyncResponse struct { // State is null until the worker has reported once. State *models.SyncState `json:"state"` Policy models.SyncPolicy `json:"policy"` + // SkipFolders is the stored skip list; the same value the policy carries, + // surfaced where PUT writes it. + SkipFolders []string `json:"skip_folders"` + // Folders is what the worker last listed on the server, INBOX first. + // Empty for Gmail and Outlook, which have no IMAP folder list. + Folders []models.SyncFolder `json:"folders"` } -// GetEmailSync reports a mailbox's sync progress and fair-use status. +// GetEmailSync reports a mailbox's sync progress, fair-use status and folder +// settings. func (h *Handler) GetEmailSync(c *gin.Context) { orgID := middleware.GetOrganizationID(c) if orgID == nil { @@ -29,5 +39,47 @@ func (h *Handler) GetEmailSync(c *gin.Context) { errx.JSON(c, xerr) return } - c.JSON(http.StatusOK, emailSyncResponse{State: state, Policy: policy}) + folders, xerr := h.EmailService.GetSyncFolders(c.Request.Context(), orgID.String(), c.Param("id")) + if xerr != nil { + errx.JSON(c, xerr) + return + } + skip := policy.SkipFolders + if skip == nil { + skip = []string{} + } + c.JSON(http.StatusOK, emailSyncResponse{State: state, Policy: policy, SkipFolders: skip, Folders: folders}) +} + +// UpdateEmailSync replaces the folders the mailbox's sync leaves alone. +// +// Naturally idempotent: the body is the desired list, so a retry converges +// on the same stored value and no Idempotency-Key is needed. +// PUT /emails/:id/sync +func (h *Handler) UpdateEmailSync(c *gin.Context) { + orgID := middleware.GetOrganizationID(c) + if orgID == nil { + errx.JSON(c, errx.ErrUnauthorized) + return + } + accountID, err := uuid.Parse(c.Param("id")) + if err != nil { + errx.Handle(c, errx.ErrUuid) + return + } + + var body models.UpdateSyncSettings + if err := c.ShouldBindJSON(&body); err != nil { + errx.Handle(c, errx.ErrInvalid) + return + } + + skip, xerr := h.EmailService.UpdateSyncSettings(c.Request.Context(), orgID.String(), accountID.String(), &body) + if xerr != nil { + errx.JSON(c, xerr) + return + } + + h.auditOrg(c, models.AuditActionUpdate, models.AuditEntityEmailAccount, &accountID, nil, map[string]string{"sync_skip_folders": "updated"}) + c.JSON(http.StatusOK, gin.H{"skip_folders": skip}) } diff --git a/internal/api/routes.go b/internal/api/routes.go index 81f4a11db..041dbe341 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -504,6 +504,7 @@ func Run( // and warmup gate, so a read-only key must not reach it. emails.POST("/:id/auth-check", m.RequireAccess(models.PermManageEmails, models.APIPermWriteEmails), middleware.RequireAPIKeyEmailAccountParam("id"), h.RefreshEmailAuthCheck) emails.GET("/:id/sync", m.RequireAccess(models.PermViewCampaigns, models.APIPermReadEmails), middleware.RequireAPIKeyEmailAccountParam("id"), h.GetEmailSync) + emails.PUT("/:id/sync", m.RequireAccess(models.PermManageEmails, models.APIPermWriteEmails), middleware.RequireAPIKeyEmailAccountParam("id"), h.UpdateEmailSync) // Which addresses the provider will let this mailbox send as, // and where its signature came from. The refresh is the only // half that calls the provider, and storing its answer is what diff --git a/internal/app/consumer/event_mailbox_delete.go b/internal/app/consumer/event_mailbox_delete.go index 3c35f0e0a..5f97f797b 100644 --- a/internal/app/consumer/event_mailbox_delete.go +++ b/internal/app/consumer/event_mailbox_delete.go @@ -22,6 +22,20 @@ func (s *JobsService) HandleMailboxDelete(ctx context.Context, e *models.JobEven CaptureError(e.UserID, e.EmailID, err) return err } + // A folder the owner excluded from sync is retired with the mail + // already stored from it; the mail itself stays at the provider. + if e.Skipped && s.UniboxRepository != nil { + n, err := s.UniboxRepository.DeleteByFolderPaths(ctx, e.EmailID, []string{e.Mailbox}) + if err != nil { + CaptureError(e.UserID, e.EmailID, err) + return err + } + log.Info(). + Str("email_id", e.EmailID.String()). + Str("folder", e.Mailbox). + Int64("messages", n). + Msg("folder excluded from sync: stored mail dropped") + } return nil } diff --git a/internal/app/consumer/event_remove_email.go b/internal/app/consumer/event_remove_email.go index 2f7d854e1..faa4549ba 100644 --- a/internal/app/consumer/event_remove_email.go +++ b/internal/app/consumer/event_remove_email.go @@ -23,7 +23,9 @@ import ( // // It also drops the local unibox entry for the removed message (best-effort). func (s *JobsService) HandleRemoveEmail(ctx context.Context, e *models.JobEventRemoveEmail) error { - if s.WarmupRepo != nil { + // A message the sync found in a folder the owner excluded is filed, not + // deleted: it is still in the mailbox, so nothing is held against anyone. + if s.WarmupRepo != nil && e.SkippedFolder == "" { if rec, _ := s.WarmupRepo.GetWarmupReceived(ctx, e.EmailID, e.ID); rec != nil { switch { case s.consumeSelfMove(ctx, e.EmailID, rec.MessageID): diff --git a/internal/app/consumer/warmup_retention_test.go b/internal/app/consumer/warmup_retention_test.go index 7c3904600..f9837083a 100644 --- a/internal/app/consumer/warmup_retention_test.go +++ b/internal/app/consumer/warmup_retention_test.go @@ -126,6 +126,21 @@ func TestRemoveEmailNeverStrikesARetiredMessage(t *testing.T) { } } +// A fresh warmup message that the sync found in a folder the owner excluded +// from sync was filed, not deleted: it is still in the mailbox, so it is not +// a strike whatever its age. +func TestRemoveEmailNeverStrikesAMessageFiledIntoASkippedFolder(t *testing.T) { + s, svc := retentionService(receivedAgo(time.Hour)) + if err := s.HandleRemoveEmail(context.Background(), &models.JobEventRemoveEmail{ + UserID: uuid.New(), EmailID: uuid.New(), ID: uuid.New(), SkippedFolder: "Warmer", + }); err != nil { + t.Fatal(err) + } + if len(svc.strikes) != 0 { + t.Fatalf("a move into a skipped folder was recorded as tampering: %v", svc.strikes) + } +} + // Gmail reports Delete as gaining the TRASH label. That is the owner's act // and is judged on the same freshness rule; a spam flag is still the graver // strike and is never subject to the window. diff --git a/internal/app/email/loader.go b/internal/app/email/loader.go index c0018dfa5..b7a8b958e 100644 --- a/internal/app/email/loader.go +++ b/internal/app/email/loader.go @@ -368,6 +368,11 @@ func (s *emailService) syncDataFor(ctx context.Context, emailID uuid.UUID) *mode OrgDailyMessages: budget.DailyMessagesPerOrg, }, } + if skip, xerr := s.emailRepository.GetSyncSkipFolders(ctx, emailID); xerr == nil { + data.Policy.SkipFolders = skip + } else { + log.Warn().Str("email_id", emailID.String()).Msg("sync skip folders lookup failed; worker syncs every folder until the next republish") + } // A pool-linked mailbox is a warmup-only mirror: no history import. if s.poolLink != nil { if linked, err := s.poolLink.GetMailboxByAccount(ctx, emailID); err == nil && linked != nil { diff --git a/internal/app/email/loader_status_test.go b/internal/app/email/loader_status_test.go index c1bef0c49..84682b741 100644 --- a/internal/app/email/loader_status_test.go +++ b/internal/app/email/loader_status_test.go @@ -19,6 +19,11 @@ type stubLoaderRepo struct { account *models.Email } +// GetSyncSkipFolders answers the loader's skip-list read with an empty list. +func (s *stubLoaderRepo) GetSyncSkipFolders(context.Context, uuid.UUID) ([]string, *errx.Error) { + return nil, nil +} + func (s *stubLoaderRepo) GetByID(ctx context.Context, emailAccountID uuid.UUID) (*models.Email, *errx.Error) { return s.account, nil } diff --git a/internal/app/email/reauth_test.go b/internal/app/email/reauth_test.go index 242c6de06..657a6c221 100644 --- a/internal/app/email/reauth_test.go +++ b/internal/app/email/reauth_test.go @@ -25,6 +25,11 @@ type stubReauthRepo struct { updated *models.UpdateEmail } +// GetSyncSkipFolders answers the loader's skip-list read with an empty list. +func (s *stubReauthRepo) GetSyncSkipFolders(context.Context, uuid.UUID) ([]string, *errx.Error) { + return nil, nil +} + func (s *stubReauthRepo) GetByID(ctx context.Context, emailAccountID uuid.UUID) (*models.Email, *errx.Error) { return s.account, nil } diff --git a/internal/app/email/service.go b/internal/app/email/service.go index 2059e4e98..440ed0e36 100644 --- a/internal/app/email/service.go +++ b/internal/app/email/service.go @@ -2,10 +2,15 @@ package email import ( "context" - "github.com/warmbly/warmbly/internal/app/instancesettings" - "golang.org/x/oauth2" + "sort" + "strings" "time" + "github.com/rs/zerolog/log" + "github.com/warmbly/warmbly/internal/app/instancesettings" + "github.com/warmbly/warmbly/internal/client/smtpimap/imap" + "golang.org/x/oauth2" + "github.com/google/uuid" "github.com/warmbly/warmbly/internal/app/cipher" "github.com/warmbly/warmbly/internal/app/feature" @@ -106,6 +111,7 @@ type EmailService interface { // operator-editable budget the mailbox syncs under. WireSyncState(repo repository.EmailSyncStateRepository) WireMailboxes(repo repository.MailboxRepository) + WireUnibox(repo repository.UniboxRepository) WireSyncBudget(src SyncBudgetSource) WirePoolLink(repo repository.PoolLinkRepository) // WireCloudLink marks managed mailboxes, which ship to the worker without a credential. @@ -129,6 +135,13 @@ type EmailService interface { // GetSyncState is the dashboard's view of a mailbox's sync: nil state when // the worker has not reported yet. GetSyncState(ctx context.Context, userID, emailID string) (*models.SyncState, models.SyncPolicy, *errx.Error) + // GetSyncFolders is the folders the sync has seen on the server, so a + // client can name one to skip. Empty for providers without folders. + GetSyncFolders(ctx context.Context, orgID, emailID string) ([]models.SyncFolder, *errx.Error) + // UpdateSyncSettings replaces the mailbox's skip list, drops the mail + // already stored from those folders, and re-ships the mailbox so the + // worker applies it on its next pass. Returns the list as stored. + UpdateSyncSettings(ctx context.Context, orgID, emailID string, body *models.UpdateSyncSettings) ([]string, *errx.Error) // StartWorkerReconciler periodically ensures every active mailbox is // assigned to a worker and loaded onto it (blocks until ctx is cancelled). StartWorkerReconciler(ctx context.Context, interval time.Duration) @@ -170,6 +183,15 @@ type emailService struct { lifecycleRepo repository.SendLifecycleRepository // accountErrors is resolved-on-reconnect error state. Optional/nil-safe. accountErrors repository.EmailAccountErrorRepository + // unibox is where a skipped folder's already-stored mail is dropped from. + // Optional: without it the worker's retirement of the folder does it. + unibox repository.UniboxRepository +} + +// WireUnibox attaches the unified inbox store, for the purge that follows a +// folder being excluded from sync. +func (s *emailService) WireUnibox(repo repository.UniboxRepository) { + s.unibox = repo } // WireAccountErrors attaches the mailbox error log so reconnects can resolve it. @@ -352,3 +374,63 @@ func (s *emailService) GetSyncState(ctx context.Context, orgID, emailID string) data := s.syncDataFor(ctx, acc.ID) return data.State, data.Policy, nil } + +// GetSyncFolders lists the IMAP folders the worker has reported for this +// mailbox, INBOX first and then by name, each with the canonical folder it +// files under. Gmail and Outlook mailboxes have no folder list here. +func (s *emailService) GetSyncFolders(ctx context.Context, orgID, emailID string) ([]models.SyncFolder, *errx.Error) { + acc, xerr := s.Get(ctx, orgID, emailID) + if xerr != nil { + return nil, xerr + } + out := []models.SyncFolder{} + userID, err := uuid.Parse(acc.UserID) + if acc.Provider != string(models.InboxProviderSMTPIMAP) || err != nil { + return out, nil + } + for _, box := range s.mailboxesFor(ctx, userID, acc.ID) { + out = append(out, models.SyncFolder{Name: box.Name, Folder: imap.CanonicalFolder(box)}) + } + sort.SliceStable(out, func(i, j int) bool { + li, lj := strings.EqualFold(out[i].Name, "INBOX"), strings.EqualFold(out[j].Name, "INBOX") + if li != lj { + return li + } + return strings.ToLower(out[i].Name) < strings.ToLower(out[j].Name) + }) + return out, nil +} + +// UpdateSyncSettings stores a normalized skip list for an IMAP mailbox. The +// purge of already-stored mail is exact-name: the worker retires each +// subfolder it stops following by name on its next pass, and the re-ship +// makes that pass apply the new list within a minute rather than at the +// reconciler's next republish. +func (s *emailService) UpdateSyncSettings(ctx context.Context, orgID, emailID string, body *models.UpdateSyncSettings) ([]string, *errx.Error) { + acc, xerr := s.Get(ctx, orgID, emailID) + if xerr != nil { + return nil, xerr + } + if body == nil { + return nil, errx.ErrInvalid + } + if acc.Provider != string(models.InboxProviderSMTPIMAP) { + return nil, errx.NewWithIdentifier(errx.BadRequest, "invalid_sync_folder", "folders can only be skipped on an IMAP mailbox") + } + folders, xerr := imap.NormalizeSkipFolders(body.SkipFolders) + if xerr != nil { + return nil, xerr + } + if xerr := s.emailRepository.SetSyncSkipFolders(ctx, orgID, emailID, folders); xerr != nil { + return nil, xerr + } + if s.unibox != nil && len(folders) > 0 { + if n, err := s.unibox.DeleteByFolderPaths(ctx, acc.ID, folders); err != nil { + log.Warn().Err(err).Str("email_id", acc.ID.String()).Msg("sync skip folders: purge of stored mail failed; the worker retires the folders on its next pass") + } else if n > 0 { + log.Info().Str("email_id", acc.ID.String()).Int64("messages", n).Msg("sync skip folders: stored mail from skipped folders dropped") + } + } + s.loadAccountBestEffort(ctx, acc.ID) + return folders, nil +} diff --git a/internal/app/email/worker_removal_test.go b/internal/app/email/worker_removal_test.go index 23f662d2a..0b4e26032 100644 --- a/internal/app/email/worker_removal_test.go +++ b/internal/app/email/worker_removal_test.go @@ -59,6 +59,11 @@ func (s *stubRemovalRepo) Update(ctx context.Context, orgID, emailAccountID stri return &models.Email{ID: id, Status: status}, nil } +// GetSyncSkipFolders answers the loader's skip-list read with an empty list. +func (s *stubRemovalRepo) GetSyncSkipFolders(context.Context, uuid.UUID) ([]string, *errx.Error) { + return nil, nil +} + func (s *stubRemovalRepo) GetByID(ctx context.Context, emailAccountID uuid.UUID) (*models.Email, *errx.Error) { if s.getErr != nil { return nil, s.getErr diff --git a/internal/app/worker/wmail/sync_imap.go b/internal/app/worker/wmail/sync_imap.go index b8e943f0c..db025c19d 100644 --- a/internal/app/worker/wmail/sync_imap.go +++ b/internal/app/worker/wmail/sync_imap.go @@ -46,6 +46,22 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { // that reached it would re-file known mail as archive under a second UID. folders = slices.DeleteFunc(folders, func(b models.Mailbox) bool { return imapVirtualFolder(&b) }) + // The folders the owner excluded leave the listing here, before renames + // are followed and before the delete sweep: one already synced is retired + // like a folder the server dropped, and one never seen is never + // baselined. They are kept aside so mail that moves into one of them can + // be recognised as filed rather than lost. + var skipped []models.Mailbox + if skip := w.skipFolders(); len(skip) > 0 { + folders = slices.DeleteFunc(folders, func(b models.Mailbox) bool { + if !imap.SkipsFolder(b, skip) { + return false + } + skipped = append(skipped, b) + return true + }) + } + // Before anything is matched by name, follow the folders whose name // changed. A rename read as a delete plus a first sighting would orphan // every message filed under the old name and re-import the folder's @@ -93,6 +109,9 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { } changed := imapFolderChanged(befBox, box, condStore) + // Decided against the cursor the previous pass left, before the + // block below moves it. + movedOut := len(skipped) > 0 && imapMovedOut(befBox, box) fullyProcessed := true var touched map[string]struct{} if changed && !stats.aborted { @@ -133,7 +152,12 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { if err := w.imapReconcileDrafts(ctx, box, touched, stats); err != nil { return err } + } else if movedOut && !stats.aborted { + if err := w.imapReconcileSkipped(ctx, box, skipped, stats); err != nil { + return err + } } + befBox.Messages = box.Messages // Without CONDSTORE a message marked read elsewhere moves no cursor, // so read state is mirrored by a periodic scan instead. It runs after @@ -166,6 +190,9 @@ outer: EmailID: w.ID, Mailbox: box.Name, UIDValidity: box.UIDValidity, + // A folder that is still on the server but now excluded takes + // the mail already stored from it along. + Skipped: slices.ContainsFunc(skipped, func(s models.Mailbox) bool { return s.Name == box.Name }), }); err != nil { return nil } @@ -199,6 +226,121 @@ outer: return nil } +// skipFolders is the owner's exclusion list as the policy in force carries +// it; a republished ADD_EMAIL changes it between passes. +func (w *WMail) skipFolders() []string { + if w.gov == nil { + return nil + } + return w.gov.Policy().SkipFolders +} + +// imapMovedOut reports whether messages left the folder since the last +// pass: the count is below the previous count plus the arrivals the UIDNEXT +// advance accounts for. An expunge moves neither cursor on every server, so +// the count is the one signal that always carries it. Only a count taken by +// this worker session counts: a folder seeded from the control plane has +// none, and the first pass baselines it. +func imapMovedOut(before, now *models.Mailbox) bool { + if before.Messages == 0 || now.UIDNext < before.UIDNext { + return false + } + arrivals := now.UIDNext - before.UIDNext + return now.Messages < before.Messages+arrivals +} + +// imapReconcileSkipped retires the platform's rows for mail that left this +// folder for one the owner excluded from sync. Nothing else that leaves a +// folder is touched: a message can go somewhere the sync does not follow +// (Gmail's All Mail) and still be wanted, so a row goes only when its +// Message-ID is found in a skipped folder. Rows checked once and found +// nowhere are remembered for the session, so a folder the owner emptied by +// hand does not cost a search per row on every later pass. +func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, skipped []models.Mailbox, stats *tickStats) *errx.MailError { + if w.SyncContext == nil || len(skipped) == 0 { + return nil + } + stored, err := w.SyncContext.ListFolderMessages(ctx, w.UserID, w.ID, box.Name, box.UIDValidity) + if err != nil { + return w.controlPlaneError(err, stats) + } + if len(stored) == 0 { + return nil + } + client := w.SmtpImapData.ImapClient + _, gen, serr := client.SelectForSyncGen(box.Name) + if serr != nil { + return serr + } + // The same guard as the drafts reconciliation: UIDs only mean anything + // inside one generation. + if gen != box.UIDValidity { + return nil + } + present, aerr := client.SearchAll() + if aerr != nil { + return aerr + } + live := make(map[uint32]struct{}, len(present)) + for _, uid := range present { + live[uint32(uid)] = struct{}{} + } + if w.skipChecked == nil || len(w.skipChecked) > imapSkipCheckedMax { + w.skipChecked = make(map[string]struct{}) + } + for _, m := range stored { + if _, ok := live[m.UID]; ok { + continue + } + if ctx.Err() != nil { + return nil + } + key := fmt.Sprintf("%s\x00%d\x00%d", box.Name, box.UIDValidity, m.UID) + if _, done := w.skipChecked[key]; done { + continue + } + folder := w.imapFindInSkipped(ctx, skipped, m.MessageID) + if folder == "" { + w.skipChecked[key] = struct{}{} + continue + } + if err := w.onEvent(models.JobEventTypeRemoveEmail, &models.JobEventRemoveEmail{ + UserID: w.UserID, + EmailID: w.ID, + ID: m.ID, + SkippedFolder: folder, + }); err != nil { + return w.controlPlaneError(err, stats) + } + w.skipChecked[key] = struct{}{} + } + return nil +} + +// imapSkipCheckedMax bounds the per-session memory of rows already looked +// for in the skipped folders; past it the memory starts over. +const imapSkipCheckedMax = 20_000 + +// imapFindInSkipped names the skipped folder holding the message, or "". +// A key the sync made up for a message without a Message-ID was never on +// the wire, so there is nothing to search for. +func (w *WMail) imapFindInSkipped(ctx context.Context, skipped []models.Mailbox, messageID string) string { + if messageID == "" || strings.HasPrefix(messageID, "no-msgid/") { + return "" + } + for i := range skipped { + uid, err := w.SmtpImapData.ImapClient.FindUIDByMessageID(ctx, skipped[i].Name, messageID) + if err != nil { + log.Debug().Err(err).Str("email_id", w.ID.String()).Str("folder", skipped[i].Name).Msg("sync: search in skipped folder failed") + continue + } + if uid != 0 { + return skipped[i].Name + } + } + return "" +} + // imapFolderChanged reports whether a folder has anything new since the // cursor we hold for it. With CONDSTORE the mod-sequence answers for new mail // AND flag changes; without it only arrivals are visible here, and flag diff --git a/internal/app/worker/wmail/sync_imap_skip_test.go b/internal/app/worker/wmail/sync_imap_skip_test.go new file mode 100644 index 000000000..bde004935 --- /dev/null +++ b/internal/app/worker/wmail/sync_imap_skip_test.go @@ -0,0 +1,199 @@ +package wmail + +import ( + "context" + "testing" + + goimap "github.com/emersion/go-imap/v2" + "github.com/google/uuid" + "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/repository" +) + +func (c *fakeImapConn) FindUIDByMessageID(_ context.Context, folder, messageID string) (uint32, error) { + c.finds++ + return c.inSkipped[folder][messageID], nil +} + +// skipBudget is fixedBudget with an owner's skip list in the policy. +type skipBudget struct { + *fixedBudget + skip []string +} + +func (b *skipBudget) Policy() models.SyncPolicy { + return normalizePolicy(models.SyncPolicy{SkipFolders: b.skip}) +} + +func mailboxEvents(events []captured, kind models.JobEventType) []captured { + var out []captured + for _, e := range events { + if e.eventType == kind { + out = append(out, e) + } + } + return out +} + +// A folder on the skip list is never baselined and never fetched, and its +// subfolders go with it; a folder that merely shares the prefix does not. +func TestImapSyncNeverBaselinesSkippedFolders(t *testing.T) { + conn := &fakeImapConn{folders: []models.Mailbox{ + {Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, Delim: "/"}, + {Name: "Warmer", UIDValidity: 9, HighestModSeq: 100, Delim: "/"}, + {Name: "Warmer/Replies", UIDValidity: 10, HighestModSeq: 100, Delim: "/"}, + {Name: "Warmer2", UIDValidity: 11, HighestModSeq: 100, Delim: "/"}, + }} + budget := &skipBudget{fixedBudget: &fixedBudget{allow: 10}, skip: []string{"warmer"}} + w, events := newIMAPTestMail(conn, budget, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100}) + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + baselined := map[string]bool{} + for _, e := range mailboxEvents(*events, models.JobEventTypeMailboxUpdate) { + baselined[e.body.(*models.JobEventMailboxUpdate).Data.Name] = true + } + if baselined["Warmer"] || baselined["Warmer/Replies"] { + t.Fatalf("a skipped folder was baselined: %v", baselined) + } + if !baselined["Warmer2"] { + t.Fatal("Warmer2 shares a prefix but is a different folder; it must be synced") + } + if len(mailboxEvents(*events, models.JobEventTypeMailboxDelete)) != 0 { + t.Fatal("nothing was tracked for the skipped folders, so nothing should be retired") + } + if len(w.SmtpImapData.Mailboxes) != 2 { + t.Fatalf("tracked %d folders, want INBOX and Warmer2", len(w.SmtpImapData.Mailboxes)) + } +} + +// A folder synced before the owner excluded it is retired like one the +// server dropped, with the marker that takes its stored mail along. +func TestImapSyncRetiresNewlySkippedFolder(t *testing.T) { + conn := &fakeImapConn{folders: []models.Mailbox{ + {Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, Delim: "/"}, + {Name: "Warmer", UIDValidity: 9, HighestModSeq: 100, Delim: "/"}, + }} + budget := &skipBudget{fixedBudget: &fixedBudget{allow: 10}, skip: []string{"Warmer"}} + w, events := newIMAPTestMail(conn, budget, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100}) + w.SmtpImapData.Mailboxes = append(w.SmtpImapData.Mailboxes, &models.Mailbox{Name: "Warmer", UIDValidity: 9, HighestModSeq: 90}) + w.tracker.setFolder("Warmer", models.SyncFolderCursor{UID: 40}) + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + deletes := mailboxEvents(*events, models.JobEventTypeMailboxDelete) + if len(deletes) != 1 { + t.Fatalf("got %d MAILBOX_DELETE events, want 1", len(deletes)) + } + del := deletes[0].body.(*models.JobEventMailboxDelete) + if del.Mailbox != "Warmer" || !del.Skipped { + t.Fatalf("retired %+v, want Warmer with the skipped marker", del) + } + if len(w.SmtpImapData.Mailboxes) != 1 || w.SmtpImapData.Mailboxes[0].Name != "INBOX" { + t.Fatalf("tracked %v, want just INBOX", w.SmtpImapData.Mailboxes) + } + if cur := w.tracker.folder("Warmer"); cur.UID != 0 { + t.Fatalf("the backfill floor for Warmer survived: %+v", cur) + } + if conn.fetches != 0 { + t.Fatalf("fetched %d batches from a skipped folder, want none", conn.fetches) + } +} + +// Mail that left a synced folder is removed only when it turns up in a +// skipped folder; mail that left for anywhere else is kept, and is not +// searched for again on the next pass. +func TestImapSyncRemovesMailMovedIntoSkippedFolder(t *testing.T) { + conn := &fakeImapConn{ + folders: []models.Mailbox{ + // Three messages were here last pass; one arrived and two left. + {Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 11, Messages: 2, Delim: "/"}, + {Name: "Warmer", UIDValidity: 9, HighestModSeq: 100, Delim: "/"}, + }, + all: []goimap.UID{8, 10}, + inSkipped: map[string]map[string]uint32{"Warmer": {"<9@fake.test>": 3}}, + } + budget := &skipBudget{fixedBudget: &fixedBudget{allow: 10}, skip: []string{"Warmer"}} + w, events := newIMAPTestMail(conn, budget, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10, Messages: 3}) + w.EmailMessageMapRepository = knownMessageMap{id: uuid.New().String()} + warmup, deleted, kept := uuid.New(), uuid.New(), uuid.New() + w.SyncContext = &fakeSyncContext{stored: map[string][]repository.StoredFolderMessage{ + "INBOX": { + {UID: 7, MessageID: "<7@fake.test>", ID: deleted}, + {UID: 8, MessageID: "<8@fake.test>", ID: kept}, + {UID: 9, MessageID: "<9@fake.test>", ID: warmup}, + }, + }} + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + removed := removeIDs(*events) + if len(removed) != 1 || removed[0] != warmup { + t.Fatalf("removed %v, want exactly the message found in Warmer (%s)", removed, warmup) + } + for _, e := range mailboxEvents(*events, models.JobEventTypeRemoveEmail) { + if got := e.body.(*models.JobEventRemoveEmail).SkippedFolder; got != "Warmer" { + t.Fatalf("removal named folder %q, want Warmer", got) + } + } + if conn.finds != 2 { + t.Fatalf("searched the skipped folder %d times, want once per vanished row (2)", conn.finds) + } + + // Next pass, nothing else changed: the row that was deleted for good is + // remembered and not searched for again. + conn.folders[0].Messages = 2 + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("second Sync: %v", err) + } + if conn.finds != 2 { + t.Fatalf("a settled row was searched for again (finds = %d)", conn.finds) + } + if len(removeIDs(*events)) != 1 { + t.Fatal("the second pass removed something") + } +} + +// Without a skip list the count check is off entirely: a folder the owner +// emptied costs no lookups and loses no rows. +func TestImapSyncLeavesVanishedMailAloneWithoutSkipList(t *testing.T) { + conn := &fakeImapConn{ + folders: []models.Mailbox{{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10, Messages: 1, Delim: "/"}}, + all: []goimap.UID{8}, + } + w, events := newIMAPTestMail(conn, &fixedBudget{allow: 10}, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10, Messages: 3}) + ctx := &fakeSyncContext{stored: map[string][]repository.StoredFolderMessage{ + "INBOX": {{UID: 7, MessageID: "<7@fake.test>", ID: uuid.New()}}, + }} + w.SyncContext = ctx + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + if ctx.calls != 0 || conn.finds != 0 || len(removeIDs(*events)) != 0 { + t.Fatalf("folder rows were reconciled without a skip list: lookups=%d finds=%d removed=%d", ctx.calls, conn.finds, len(removeIDs(*events))) + } +} + +func TestImapMovedOut(t *testing.T) { + cases := []struct { + name string + before, now models.Mailbox + want bool + }{ + {"nothing changed", models.Mailbox{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 3, UIDNext: 10}, false}, + {"one arrived", models.Mailbox{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 4, UIDNext: 11}, false}, + {"one left", models.Mailbox{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 2, UIDNext: 10}, true}, + {"one arrived and one left", models.Mailbox{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 3, UIDNext: 11}, true}, + {"no baseline from this session", models.Mailbox{Messages: 0, UIDNext: 10}, models.Mailbox{Messages: 2, UIDNext: 10}, false}, + {"cursor went backwards", models.Mailbox{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 1, UIDNext: 5}, false}, + } + for _, tc := range cases { + if got := imapMovedOut(&tc.before, &tc.now); got != tc.want { + t.Errorf("%s: imapMovedOut = %v, want %v", tc.name, got, tc.want) + } + } +} diff --git a/internal/app/worker/wmail/sync_imap_test.go b/internal/app/worker/wmail/sync_imap_test.go index 6409ae46c..12654352f 100644 --- a/internal/app/worker/wmail/sync_imap_test.go +++ b/internal/app/worker/wmail/sync_imap_test.go @@ -36,6 +36,10 @@ type fakeImapConn struct { // selectGen, when non-zero, is the UIDVALIDITY SELECT reports, which the // reconciliation compares against the one the listing gave it. selectGen uint32 + // inSkipped is what FindUIDByMessageID answers per folder name, and + // finds counts how often it was asked. + inSkipped map[string]map[string]uint32 + finds int } func (c *fakeImapConn) Folders() ([]models.Mailbox, *errx.MailError) { return c.folders, nil } diff --git a/internal/app/worker/wmail/wmail.go b/internal/app/worker/wmail/wmail.go index c1d3c9212..ed6277b19 100644 --- a/internal/app/worker/wmail/wmail.go +++ b/internal/app/worker/wmail/wmail.go @@ -98,6 +98,10 @@ type WMail struct { // flagScan is the previous flag snapshot per folder name, used only on // IMAP servers without CONDSTORE, which cannot say what changed. flagScan map[string]*folderFlagScan + // skipChecked remembers, for this session, the stored rows that left a + // synced folder and were looked for in the skipped folders without being + // found, so they are not searched for again on every pass. + skipChecked map[string]struct{} // transportFailures counts consecutive passes that could not reach the // mail server, which paces the retry and keeps one outage to one warning. transportFailures int diff --git a/internal/client/smtpimap/imap/folders.go b/internal/client/smtpimap/imap/folders.go index c2ae4c44e..07d70e342 100644 --- a/internal/client/smtpimap/imap/folders.go +++ b/internal/client/smtpimap/imap/folders.go @@ -2,8 +2,10 @@ package imap import ( "errors" + "fmt" "sort" "strings" + "unicode/utf8" "github.com/emersion/go-imap/v2" "github.com/rs/zerolog/log" @@ -41,6 +43,7 @@ func (c *Client) foldersCapped(limit int) ([]models.Mailbox, *errx.MailError) { status := &imap.StatusOptions{ UIDValidity: true, UIDNext: true, + NumMessages: true, // Asking a server without CONDSTORE for HIGHESTMODSEQ is a BAD. HighestModSeq: caps.Has(imap.CapCondStore), } @@ -101,6 +104,9 @@ func (c *Client) foldersCapped(limit int) ([]models.Mailbox, *errx.MailError) { box.UIDValidity = st.UIDValidity box.UIDNext = uint32(st.UIDNext) box.HighestModSeq = st.HighestModSeq + if st.NumMessages != nil { + box.Messages = *st.NumMessages + } resp = append(resp, box) } @@ -262,6 +268,86 @@ func BackfillEligible(box models.Mailbox) bool { return true } +// SkipsFolder reports whether box is one the mailbox owner asked the sync to +// leave alone. A name in skip matches the folder listed under it and every +// folder below it, compared the way servers compare names: case does not +// count. INBOX and the special folders (by attribute or by name) never match, +// whatever the list says, because a sync without them is a broken mailbox +// rather than a quieter one. +func SkipsFolder(box models.Mailbox, skip []string) bool { + if len(skip) == 0 || !skippableFolder(box) { + return false + } + for _, s := range skip { + s = strings.TrimSpace(s) + if s == "" || strings.EqualFold(s, "INBOX") { + continue + } + if strings.EqualFold(box.Name, s) { + return true + } + if box.Delim != "" && hasPrefixFold(box.Name, s+box.Delim) { + return true + } + } + return false +} + +// skippableFolder is false for INBOX and for any folder the mailbox needs +// whole: a special-use attribute or a recognised special name. +func skippableFolder(box models.Mailbox) bool { + if strings.EqualFold(strings.TrimSpace(box.Name), "INBOX") { + return false + } + for _, a := range box.Attrs { + switch strings.ToLower(a) { + case "\\inbox", "\\sent", "\\drafts", "\\junk", "\\trash", "\\archive", "\\all", "\\flagged", "\\important": + return false + } + } + return CanonicalFolder(box) == models.FolderInbox +} + +// NormalizeSkipFolders is the write-side check on a skip list: trimmed, +// deduplicated without regard to case, bounded in count and length, no +// control characters, and none of the names the sync must keep. The result +// is what gets stored; the error names the first entry refused. +func NormalizeSkipFolders(names []string) ([]string, *errx.Error) { + out := make([]string, 0, len(names)) + seen := make(map[string]struct{}, len(names)) + for _, raw := range names { + name := strings.TrimSpace(raw) + if name == "" { + continue + } + if utf8.RuneCountInString(name) > config.SyncSkipFolderNameMax { + return nil, skipFolderError(name, "is longer than the folder name limit") + } + for _, r := range name { + if r < 0x20 || r == 0x7f { + return nil, skipFolderError(name, "contains a control character") + } + } + if !skippableFolder(models.Mailbox{Name: name}) { + return nil, skipFolderError(name, "is a folder the sync always follows") + } + key := strings.ToLower(name) + if _, dup := seen[key]; dup { + continue + } + seen[key] = struct{}{} + out = append(out, name) + } + if len(out) > config.SyncSkipFoldersMax { + return nil, errx.NewWithIdentifier(errx.BadRequest, "invalid_sync_folder", fmt.Sprintf("at most %d folders can be skipped", config.SyncSkipFoldersMax)) + } + return out, nil +} + +func skipFolderError(name, why string) *errx.Error { + return errx.NewWithIdentifier(errx.BadRequest, "invalid_sync_folder", fmt.Sprintf("folder %q %s", name, why)) +} + // CanonicalFolder maps an IMAP folder to the canonical unibox folder. // Special-use attributes are authoritative, with a name fallback for servers // that do not advertise them; unrecognized user folders file as inbox so diff --git a/internal/client/smtpimap/imap/folders_skip_test.go b/internal/client/smtpimap/imap/folders_skip_test.go new file mode 100644 index 000000000..cf348ff01 --- /dev/null +++ b/internal/client/smtpimap/imap/folders_skip_test.go @@ -0,0 +1,80 @@ +package imap + +import ( + "strings" + "testing" + + "github.com/warmbly/warmbly/internal/config" + "github.com/warmbly/warmbly/internal/models" +) + +func TestSkipsFolder(t *testing.T) { + skip := []string{"Warmer", "Clients/Acme", " inbox "} + cases := []struct { + name string + box models.Mailbox + want bool + }{ + {"exact", models.Mailbox{Name: "Warmer", Delim: "/"}, true}, + {"case does not count", models.Mailbox{Name: "WARMER", Delim: "/"}, true}, + {"subfolder", models.Mailbox{Name: "Warmer/Replies", Delim: "/"}, true}, + {"subfolder under a dot server", models.Mailbox{Name: "Warmer.Replies", Delim: "."}, true}, + {"nested entry", models.Mailbox{Name: "Clients/Acme/2026", Delim: "/"}, true}, + {"prefix without a delimiter is a different folder", models.Mailbox{Name: "Warmer2", Delim: "/"}, false}, + {"no delimiter reported means exact only", models.Mailbox{Name: "Warmer/Replies", Delim: ""}, false}, + {"another folder", models.Mailbox{Name: "Receipts", Delim: "/"}, false}, + {"INBOX is never skipped, even when listed", models.Mailbox{Name: "INBOX", Delim: "/"}, false}, + {"an INBOX entry does not take the inbox's subfolders", models.Mailbox{Name: "INBOX/Work", Delim: "/"}, false}, + {"special-use attribute wins over the name", models.Mailbox{Name: "Warmer", Delim: "/", Attrs: []string{"\\Sent"}}, false}, + {"special name wins", models.Mailbox{Name: "Sent", Delim: "/"}, false}, + } + for _, tc := range cases { + if got := SkipsFolder(tc.box, skip); got != tc.want { + t.Errorf("%s: SkipsFolder(%q) = %v, want %v", tc.name, tc.box.Name, got, tc.want) + } + } + if SkipsFolder(models.Mailbox{Name: "Warmer"}, nil) { + t.Error("an empty list skips nothing") + } +} + +func TestNormalizeSkipFolders(t *testing.T) { + got, xerr := NormalizeSkipFolders([]string{" Warmer ", "", "warmer", "Clients/Acme", "INBOX/Newsletters"}) + if xerr != nil { + t.Fatalf("unexpected refusal: %v", xerr) + } + want := []string{"Warmer", "Clients/Acme", "INBOX/Newsletters"} + if strings.Join(got, "|") != strings.Join(want, "|") { + t.Fatalf("normalized %v, want %v", got, want) + } + + refused := [][]string{ + {"INBOX"}, + {"inbox"}, + {"Sent"}, + {"INBOX.Drafts"}, + {"Junk"}, + {"[Gmail]/Trash"}, + {"Archive"}, + {"bad\x01name"}, + {strings.Repeat("x", config.SyncSkipFolderNameMax+1)}, + } + for _, in := range refused { + if _, xerr := NormalizeSkipFolders(in); xerr == nil { + t.Errorf("NormalizeSkipFolders(%q) accepted, want a refusal", in) + } else if xerr.Identifier != "invalid_sync_folder" { + t.Errorf("NormalizeSkipFolders(%q) refused as %q, want invalid_sync_folder", in, xerr.Identifier) + } + } + + many := make([]string, config.SyncSkipFoldersMax+1) + for i := range many { + many[i] = "Folder" + strings.Repeat("x", i) + } + if _, xerr := NormalizeSkipFolders(many); xerr == nil { + t.Error("a list over the cap was accepted") + } + if got, _ := NormalizeSkipFolders(nil); got == nil || len(got) != 0 { + t.Errorf("nil in, want an empty list out, got %#v", got) + } +} diff --git a/internal/config/constants.go b/internal/config/constants.go index 454141d86..b3dd23d29 100644 --- a/internal/config/constants.go +++ b/internal/config/constants.go @@ -95,6 +95,8 @@ const ( SyncBackfillPerMinute = 240 // backfill pacing per mailbox SyncFloodPerHour = 5_000 // new live messages observed in one hour that mark a mailbox as flooding SyncThrottleEscalationDays = 3 // throttled UTC days out of the last 7 that deactivate a mailbox + SyncSkipFoldersMax = 50 // folders one mailbox may exclude from sync + SyncSkipFolderNameMax = 255 // characters in one excluded folder name // Forms. Funnel events feed analytics ranges up to 90 days, so the default // window keeps double coverage. Operator-editable under Instance settings. diff --git a/internal/infrastructure/db/migrations/000196_email_sync_skip_folders.down.sql b/internal/infrastructure/db/migrations/000196_email_sync_skip_folders.down.sql new file mode 100644 index 000000000..201998d7a --- /dev/null +++ b/internal/infrastructure/db/migrations/000196_email_sync_skip_folders.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE public.email_accounts + DROP COLUMN IF EXISTS sync_skip_folders; diff --git a/internal/infrastructure/db/migrations/000196_email_sync_skip_folders.up.sql b/internal/infrastructure/db/migrations/000196_email_sync_skip_folders.up.sql new file mode 100644 index 000000000..cf87c43bb --- /dev/null +++ b/internal/infrastructure/db/migrations/000196_email_sync_skip_folders.up.sql @@ -0,0 +1,14 @@ +-- A mailbox owner can name folders the sync leaves alone. +-- +-- The IMAP sync follows every folder the server lists, and a folder it does +-- not recognise files as inbox so the mail in it stays visible. A folder a +-- third-party tool fills with its own machine traffic then lands in the +-- unified inbox, spends the mailbox's sync budget, and is classified like a +-- reply. The names here are matched against the server's listing and the +-- folders they name (and their subfolders) are never opened. +-- +-- email_accounts.sync_skip_folders: folder names as the server lists them. +-- Empty means everything is synced. The special folders (inbox, sent, +-- drafts, spam, trash, archive) cannot be named here. +ALTER TABLE public.email_accounts + ADD COLUMN sync_skip_folders text[] NOT NULL DEFAULT '{}'; diff --git a/internal/models/event_w_emails.go b/internal/models/event_w_emails.go index 479cbbdb2..b791aa9fc 100644 --- a/internal/models/event_w_emails.go +++ b/internal/models/event_w_emails.go @@ -20,6 +20,10 @@ type JobEventRemoveEmail struct { UserID uuid.UUID `json:"user_id" avro:"user_id"` EmailID uuid.UUID `json:"email_id" avro:"email_id"` ID uuid.UUID `json:"id" avro:"id"` + // SkippedFolder is set when the message was found in a folder the owner + // excluded from sync: it still exists in the mailbox, so the removal is + // filing, not deletion. + SkippedFolder string `json:"skipped_folder,omitempty" avro:"skipped_folder"` } type JobEventFlags struct { diff --git a/internal/models/event_w_mailbox.go b/internal/models/event_w_mailbox.go index a426222ee..d82440729 100644 --- a/internal/models/event_w_mailbox.go +++ b/internal/models/event_w_mailbox.go @@ -18,6 +18,9 @@ type JobEventMailboxDelete struct { Mailbox string `json:"mailbox,omitempty" avro:"mailbox"` // UIDValidity is that legacy fallback and nothing else. UIDValidity uint32 `json:"uid_validity" avro:"uid_validity"` + // Skipped means the folder is still on the server but the owner excluded + // it from sync, so the mail already stored from it is retired as well. + Skipped bool `json:"skipped,omitempty" avro:"skipped"` } // JobEventMailboxRename is a folder that kept its UIDVALIDITY under a new diff --git a/internal/models/mailbox.go b/internal/models/mailbox.go index 9d09cff88..498c3cd1c 100644 --- a/internal/models/mailbox.go +++ b/internal/models/mailbox.go @@ -22,6 +22,11 @@ type Mailbox struct { // ("/" on Gmail, "." on many Dovecots). Empty when the server reported // none, where the leaf is guessed instead. Delim string `json:"delim,omitempty" avro:"delim"` + // Messages is the folder's message count at the last listing. Held by the + // worker between passes and not persisted: a drop that the arrivals do + // not explain is what makes a pass look for mail moved into a folder the + // sync does not follow. + Messages uint32 `json:"messages,omitempty" avro:"messages"` UpdatedAt time.Time `json:"updated_at" avro:"updated_at"` } diff --git a/internal/models/sync.go b/internal/models/sync.go index aeb592bb9..c89bb135f 100644 --- a/internal/models/sync.go +++ b/internal/models/sync.go @@ -21,6 +21,25 @@ type SyncPolicy struct { // OrgDailyMessages caps new plus backfilled messages stored across the // whole organization per UTC day. OrgDailyMessages int `json:"org_daily_messages" avro:"org_daily_messages"` + // SkipFolders names the folders the sync leaves alone, as the server + // lists them; each also covers its subfolders. Per mailbox, unlike the + // budgets above, and only meaningful on IMAP. + SkipFolders []string `json:"skip_folders,omitempty" avro:"skip_folders"` +} + +// SyncFolder is one folder the sync has seen on the server, as GET +// /emails/:id/sync reports it: the name to use in skip_folders and the +// canonical folder it files under, so a client can tell which ones are the +// special folders that cannot be skipped. +type SyncFolder struct { + Name string `json:"name"` + Folder string `json:"folder"` +} + +// UpdateSyncSettings is the body of PUT /emails/:id/sync. The list is the +// desired state, so a retry converges. +type UpdateSyncSettings struct { + SkipFolders []string `json:"skip_folders"` } // SyncBackfillStatus is where the initial import stands. diff --git a/internal/repository/pg_email.go b/internal/repository/pg_email.go index 8f473041a..70b4f08c2 100644 --- a/internal/repository/pg_email.go +++ b/internal/repository/pg_email.go @@ -115,6 +115,13 @@ type EmailRepository interface { SetWarmupLifecycle(ctx context.Context, orgID, emailAccountID, action string) (*models.Email, *errx.Error) UpdateTrackingDomain(ctx context.Context, orgID, emailAccountID, domain string, verified bool, verifiedAt *time.Time) *errx.Error UpdateTrackDirectMail(ctx context.Context, orgID, emailAccountID string, enabled bool) *errx.Error + // GetSyncSkipFolders is the folders this mailbox's sync leaves alone. + // Read without a tenant predicate by the loader, which ships it to the + // worker inside the mailbox's sync policy. + GetSyncSkipFolders(ctx context.Context, emailAccountID uuid.UUID) ([]string, *errx.Error) + // SetSyncSkipFolders replaces that list, scoped by organization like every + // other mailbox setting a workspace admin may change. + SetSyncSkipFolders(ctx context.Context, orgID, emailAccountID string, folders []string) *errx.Error // ListOrganizationIDs names every workspace with a mailbox, for sweeps that // run per workspace rather than per event. ListOrganizationIDs(ctx context.Context) ([]uuid.UUID, error) @@ -2121,6 +2128,46 @@ func (r *emailRepository) UpdateTrackDirectMail(ctx context.Context, orgID, emai return nil } +// GetSyncSkipFolders reads the mailbox's skip list; an empty list for a +// mailbox that never set one. +func (r *emailRepository) GetSyncSkipFolders(ctx context.Context, emailAccountID uuid.UUID) ([]string, *errx.Error) { + query := `SELECT sync_skip_folders FROM email_accounts WHERE id = $1` + var folders []string + if err := r.DB.QueryRow(ctx, query, emailAccountID).Scan(&folders); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return nil, errx.ErrNotFound + } + db.CaptureError(err, query, []any{emailAccountID}, "query") + return nil, errx.InternalError() + } + return folders, nil +} + +// SetSyncSkipFolders stores the skip list as given; the caller has already +// normalized it. A nil slice would bind as SQL NULL against a NOT NULL +// column, so an empty list is written as an empty array. +func (r *emailRepository) SetSyncSkipFolders(ctx context.Context, orgID, emailAccountID string, folders []string) *errx.Error { + if folders == nil { + folders = []string{} + } + query := ` + UPDATE email_accounts + SET sync_skip_folders = $1, updated_at = NOW() + WHERE organization_id = $2 AND id = $3 + ` + params := []any{folders, orgID, emailAccountID} + + cmd, err := r.DB.Exec(ctx, query, params...) + if err != nil { + db.CaptureError(err, query, params, "exec") + return errx.InternalError() + } + if cmd.RowsAffected() == 0 { + return errx.ErrNotFound + } + return nil +} + // ListOrganizationIDs returns every workspace that has at least one mailbox. func (r *emailRepository) ListOrganizationIDs(ctx context.Context) ([]uuid.UUID, error) { rows, err := r.DB.Query(ctx, `SELECT DISTINCT organization_id FROM email_accounts WHERE organization_id IS NOT NULL`) diff --git a/internal/repository/pg_unibox.go b/internal/repository/pg_unibox.go index b5ad0d7f9..a42c2043c 100644 --- a/internal/repository/pg_unibox.go +++ b/internal/repository/pg_unibox.go @@ -69,6 +69,10 @@ type UniboxRepository interface { // are left out: there is nothing to relay through. SeenRelayTargets(ctx context.Context, orgID uuid.UUID, ids []uuid.UUID) ([]models.SeenRelayTarget, error) Delete(ctx context.Context, userID, id uuid.UUID) error + // DeleteByFolderPaths drops every stored message one mailbox synced from + // the named source folders, for folders the owner has excluded from sync. + // Exact names only; the worker names each subfolder it retires itself. + DeleteByFolderPaths(ctx context.Context, emailID uuid.UUID, folderPaths []string) (int64, error) ListWarmupReviewCandidates(ctx context.Context, afterID uuid.UUID, limit int) ([]models.JobEventNewEmail, error) // ListUnprocessedCampaignReplies pages inbound messages that reply // processing never claimed and that look like campaign replies: they @@ -918,6 +922,19 @@ func (r *uniboxRepository) Delete(ctx context.Context, userID, id uuid.UUID) err return tx.Commit(ctx) } +// DeleteByFolderPaths removes the mirror rows for whole source folders. The +// mail stays where it is at the provider; only the platform's copy goes. +func (r *uniboxRepository) DeleteByFolderPaths(ctx context.Context, emailID uuid.UUID, folderPaths []string) (int64, error) { + if len(folderPaths) == 0 { + return 0, nil + } + tag, err := r.db.Exec(ctx, `DELETE FROM unibox_emails WHERE email_id = $1 AND folder_path = ANY($2)`, emailID, folderPaths) + if err != nil { + return 0, err + } + return tag.RowsAffected(), nil +} + // queryPreviewList executes a query returning preview rows with limit+1 pagination. func (r *uniboxRepository) queryPreviewList(ctx context.Context, query string, args []any, limit int) (*models.MailSearchResult, error) { rows, err := r.db.Query(ctx, query, args...) diff --git a/skills/warmbly-api/SKILL.md b/skills/warmbly-api/SKILL.md index 3ab204124..7305b07fe 100644 --- a/skills/warmbly-api/SKILL.md +++ b/skills/warmbly-api/SKILL.md @@ -38,7 +38,7 @@ Run `warmblyctl --help` for subcommands and `warmblyctl | `me` | Identity and granted scopes | | `campaign` | list, get, create, update, delete, steps, senders, preflight, start, stop, test-email, logs, plan, pause-lead / resume-lead | | `contact` | list (search), get, lookup, create, update, delete, notes, timeline, import, export | -| `mailbox` | list, get, update, delete, auth-check, sync, identity, refresh-identity, behavior, verify, send, warmup-start/pause/resume/stop/status | +| `mailbox` | list, get, update, delete, auth-check, sync, skip-folders, identity, refresh-identity, behavior, verify, send, warmup-start/pause/resume/stop/status | | `inbox` | list, count, thread, seen, reply, compose, agent drafts, scheduled sends | | `analytics` | dashboard, deliverability, warmup, accounts, campaigns, usage, audit-logs | | `settings` | outreach and suppression settings | diff --git a/skills/warmbly-cli/SKILL.md b/skills/warmbly-cli/SKILL.md index 4577f1f0c..0bface539 100644 --- a/skills/warmbly-cli/SKILL.md +++ b/skills/warmbly-cli/SKILL.md @@ -74,7 +74,7 @@ gives the arguments and flags. Ids are positional, not flags. | `status` | one call for "what is happening": mailboxes needing attention, what is sending, what is unread | | `campaign` | list, view, create, edit, delete, steps, senders, segments, preflight, test, start, stop, logs, plan, pause-lead / resume-lead | | `contact` | list, view, create, edit, delete, lookup, timeline, emails, notes, import, export, verify | -| `mailbox` | list, view, edit, check, sync, identity, refresh-identity, behavior, warmup, hold, release, send | +| `mailbox` | list, view, edit, check, sync, skip-folders, identity, refresh-identity, behavior, warmup, hold, release, send | | `inbox` | list, view, thread, read, reply, compose, drafts, scheduled, snooze | | `suppression` | the list of addresses and domains that get no campaign mail | | `segment`, `template`, `automation`, `form` | audiences, reply templates, automations, lead capture | diff --git a/web/src/components/app/emails/InboxDetails.tsx b/web/src/components/app/emails/InboxDetails.tsx index bff357ab5..95dec426c 100644 --- a/web/src/components/app/emails/InboxDetails.tsx +++ b/web/src/components/app/emails/InboxDetails.tsx @@ -642,7 +642,7 @@ function OverviewTab({ status, loading, mailbox }: { status?: import("@/lib/api/ {/* Sync: import progress and fair-use status */} - + {/* Key stats */}

diff --git a/web/src/components/app/emails/SyncStatusCard.tsx b/web/src/components/app/emails/SyncStatusCard.tsx index c8e5a6ebf..1b3f1a7d8 100644 --- a/web/src/components/app/emails/SyncStatusCard.tsx +++ b/web/src/components/app/emails/SyncStatusCard.tsx @@ -1,7 +1,14 @@ +import { useState } from "react"; import { motion } from "framer-motion"; -import { CheckCircle2Icon, DownloadIcon, HourglassIcon, RefreshCwIcon } from "lucide-react"; +import { CheckCircle2Icon, DownloadIcon, HourglassIcon, PlusIcon, RefreshCwIcon } from "lucide-react"; +import toast from "react-hot-toast"; +import { CheckSquare } from "@/components/ui/check-square"; +import { TextInput } from "@/components/ui/field"; +import type { AppError } from "@/lib/api/client/normalizeError"; import useSync from "@/lib/api/hooks/app/emails/useSync"; -import type { SyncThrottleReason } from "@/lib/api/models/app/emails/SyncState"; +import useUpdateSyncSkipFolders from "@/lib/api/hooks/app/emails/useUpdateSyncSkipFolders"; +import type { SyncFolder, SyncThrottleReason } from "@/lib/api/models/app/emails/SyncState"; +import buildError from "@/lib/helper/buildError"; import { cn } from "@/lib/utils"; // Sync card in the mailbox drawer: what the initial import has done, whether @@ -35,7 +42,98 @@ function until(iso: string): string { : d.toLocaleString([], { weekday: "short", hour: "2-digit", minute: "2-digit" }); } -export default function SyncStatusCard({ mailboxId }: { mailboxId: string }) { +// The folders a client may offer to skip: everything the worker listed +// except INBOX and the special folders, which the sync always follows. +function skippable(folders: SyncFolder[]): string[] { + return folders.filter((f) => f.folder === "inbox" && f.name.toUpperCase() !== "INBOX").map((f) => f.name); +} + +// Folders the owner excluded from sync, with the ones the server lists as +// the choices. A skipped folder leaves the listing once the worker stops +// following it, so the rows are the union of both, and a name the listing +// does not have yet (a folder not yet seen, or one on a mailbox that has not +// synced) can be typed in. +function SkipFoldersSection({ mailboxId, listed, skipped }: { mailboxId: string; listed: string[]; skipped: string[] }) { + const mutation = useUpdateSyncSkipFolders(mailboxId); + const [draft, setDraft] = useState(""); + + const isSkipped = (name: string) => skipped.some((s) => s.toLowerCase() === name.toLowerCase()); + const rows = [...listed, ...skipped.filter((s) => !listed.some((l) => l.toLowerCase() === s.toLowerCase()))]; + + const save = async (next: string[]) => { + try { + await mutation.mutateAsync(next); + } catch (e) { + toast.error(buildError(e as AppError)); + } + }; + const toggle = (name: string) => + save(isSkipped(name) ? skipped.filter((s) => s.toLowerCase() !== name.toLowerCase()) : [...skipped, name]); + const add = () => { + const name = draft.trim(); + if (!name) return; + setDraft(""); + if (isSkipped(name)) return; + void save([...skipped, name]); + }; + + return ( +
+
Folders not synced
+

+ Mail in a folder ticked here never reaches Warmbly, and what was already imported from it is removed. Use it + for a folder another tool fills, such as a second warmup service. Inbox, sent, drafts, spam, trash and archive + always sync. +

+ {rows.length > 0 && ( +
    + {rows.map((name) => ( +
  • + +
    • + ))} +
+ )} +
+ { + if (e.key === "Enter") { + e.preventDefault(); + add(); + } + }} + className="flex-1" + /> + +
+
+ ); +} + +export default function SyncStatusCard({ mailboxId, provider }: { mailboxId: string; provider?: string }) { const sync = useSync(mailboxId); const state = sync.data?.state ?? null; const policy = sync.data?.policy; @@ -129,6 +227,14 @@ export default function SyncStatusCard({ mailboxId }: { mailboxId: string }) { synced. Renaming one of them on your mail server clears this.

)} + + {provider === "smtp_imap" && ( + + )}
); } diff --git a/web/src/lib/api/client/app/emails/updateSync.ts b/web/src/lib/api/client/app/emails/updateSync.ts new file mode 100644 index 000000000..0d85b04a6 --- /dev/null +++ b/web/src/lib/api/client/app/emails/updateSync.ts @@ -0,0 +1,12 @@ +import Request from "../../Request"; + +// Replaces the folders a mailbox's sync leaves alone. The list is the +// desired state, so a retry converges. +export default async function updateSync(id: string, skipFolders: string[]): Promise<{ skip_folders: string[] }> { + return await Request<{ skip_folders: string[] }>({ + method: "PUT", + url: `/emails/${id}/sync`, + data: { skip_folders: skipFolders }, + authorization: true, + }); +} diff --git a/web/src/lib/api/hooks/app/emails/useUpdateSyncSkipFolders.ts b/web/src/lib/api/hooks/app/emails/useUpdateSyncSkipFolders.ts new file mode 100644 index 000000000..a051c6581 --- /dev/null +++ b/web/src/lib/api/hooks/app/emails/useUpdateSyncSkipFolders.ts @@ -0,0 +1,19 @@ +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import updateSync from "@/lib/api/client/app/emails/updateSync"; +import type EmailSync from "@/lib/api/models/app/emails/SyncState"; + +export default function useUpdateSyncSkipFolders(id: string) { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: (skipFolders: string[]) => updateSync(id, skipFolders), + onSuccess: (data) => { + queryClient.setQueryData(["emails", id, "sync"], (prev) => + prev ? { ...prev, skip_folders: data.skip_folders, policy: { ...prev.policy, skip_folders: data.skip_folders } } : prev, + ); + // Mail already stored from a newly skipped folder is dropped on + // the server, so every inbox list is stale. + queryClient.invalidateQueries({ queryKey: ["unibox"] }); + }, + }); +} diff --git a/web/src/lib/api/models/app/emails/SyncState.ts b/web/src/lib/api/models/app/emails/SyncState.ts index 9bf118d15..fd0accda5 100644 --- a/web/src/lib/api/models/app/emails/SyncState.ts +++ b/web/src/lib/api/models/app/emails/SyncState.ts @@ -32,9 +32,21 @@ export interface SyncPolicy { backfill_messages: number; daily_messages: number; org_daily_messages: number; + /** Folders the sync leaves alone, as the server lists them. IMAP only. */ + skip_folders?: string[]; +} + +/** One folder the sync has seen on the server, with the canonical folder it files under. */ +export interface SyncFolder { + name: string; + folder: "inbox" | "sent" | "drafts" | "spam" | "trash" | "archive" | string; } export default interface EmailSync { state: SyncState | null; policy: SyncPolicy; + /** The stored skip list; PUT /emails/:id/sync replaces it. */ + skip_folders: string[]; + /** What the worker last listed, INBOX first. Empty for Gmail and Outlook. */ + folders: SyncFolder[]; } From ad353b385a39a5b15038aeb5b9895728f6af0e28 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 22 Sep 2026 05:20:06 -0700 Subject: [PATCH 6/8] feat: key the IMAP departure check on the previous listing's count and UIDNEXT per folder instead of the stored cursor, since a walked folder's cursor now advances to the SELECT view and a server whose view lags its STATUS would read as departures every pass; the fake listing in the worker tests returns a copy so a second pass still sees the skipped folder --- internal/app/worker/wmail/sync_imap.go | 40 +++++++--- .../app/worker/wmail/sync_imap_skip_test.go | 80 +++++++++++++++---- internal/app/worker/wmail/sync_imap_test.go | 7 +- internal/app/worker/wmail/wmail.go | 6 ++ 4 files changed, 103 insertions(+), 30 deletions(-) diff --git a/internal/app/worker/wmail/sync_imap.go b/internal/app/worker/wmail/sync_imap.go index 81ffac6e7..bcf453405 100644 --- a/internal/app/worker/wmail/sync_imap.go +++ b/internal/app/worker/wmail/sync_imap.go @@ -80,6 +80,10 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { for i := range folders { box := &folders[i] + // The listing is remembered before anything is decided from it, so + // the departure check below reads the previous pass, never this one. + prevListing, listedBefore := w.listed[box.Name] + w.rememberListing(box) befBox := w.SmtpImapData.FindPair(box) if befBox == nil { // First sight: baseline. Live sync starts from this cursor; the @@ -112,9 +116,7 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { } changed := imapFolderChanged(befBox, box, condStore) - // Decided against the cursor the previous pass left, before the - // block below moves it. - movedOut := len(skipped) > 0 && imapMovedOut(befBox, box) + movedOut := len(skipped) > 0 && listedBefore && imapMovedOut(prevListing, box) fullyProcessed := true var touched map[string]struct{} var view imap.Selected @@ -164,7 +166,6 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { return err } } - befBox.Messages = box.Messages // Without CONDSTORE a message marked read elsewhere moves no cursor, // so read state is mirrored by a periodic scan instead. It runs after @@ -209,6 +210,7 @@ outer: if len(deleted) > 0 { for _, name := range deleted { delete(w.flagScan, name) + delete(w.listed, name) // The backfill floor goes with the folder. A name is reusable, // and a floor left behind would be inherited by whatever is // created under it next. @@ -242,14 +244,28 @@ func (w *WMail) skipFolders() []string { return w.gov.Policy().SkipFolders } -// imapMovedOut reports whether messages left the folder since the last -// pass: the count is below the previous count plus the arrivals the UIDNEXT -// advance accounts for. An expunge moves neither cursor on every server, so -// the count is the one signal that always carries it. Only a count taken by -// this worker session counts: a folder seeded from the control plane has -// none, and the first pass baselines it. -func imapMovedOut(before, now *models.Mailbox) bool { - if before.Messages == 0 || now.UIDNext < before.UIDNext { +// imapListed is what one listing said about a folder: the two numbers the +// departure check compares between passes. +type imapListed struct { + Messages uint32 + UIDNext uint32 +} + +func (w *WMail) rememberListing(box *models.Mailbox) { + if w.listed == nil { + w.listed = make(map[string]imapListed) + } + w.listed[box.Name] = imapListed{Messages: box.Messages, UIDNext: box.UIDNext} +} + +// imapMovedOut reports whether messages left the folder between two +// listings: the count is below the previous count plus the arrivals the +// UIDNEXT advance accounts for. An expunge moves neither cursor on every +// server, so the count is the one signal that always carries it. Both +// numbers come from the listing, never from the SELECT view a walked +// folder's cursor advances to. +func imapMovedOut(before imapListed, now *models.Mailbox) bool { + if now.UIDNext < before.UIDNext { return false } arrivals := now.UIDNext - before.UIDNext diff --git a/internal/app/worker/wmail/sync_imap_skip_test.go b/internal/app/worker/wmail/sync_imap_skip_test.go index bde004935..f18547696 100644 --- a/internal/app/worker/wmail/sync_imap_skip_test.go +++ b/internal/app/worker/wmail/sync_imap_skip_test.go @@ -116,7 +116,9 @@ func TestImapSyncRemovesMailMovedIntoSkippedFolder(t *testing.T) { inSkipped: map[string]map[string]uint32{"Warmer": {"<9@fake.test>": 3}}, } budget := &skipBudget{fixedBudget: &fixedBudget{allow: 10}, skip: []string{"Warmer"}} - w, events := newIMAPTestMail(conn, budget, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10, Messages: 3}) + w, events := newIMAPTestMail(conn, budget, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10}) + // The previous pass of this session listed three messages. + w.rememberListing(&models.Mailbox{Name: "INBOX", Messages: 3, UIDNext: 10}) w.EmailMessageMapRepository = knownMessageMap{id: uuid.New().String()} warmup, deleted, kept := uuid.New(), uuid.New(), uuid.New() w.SyncContext = &fakeSyncContext{stored: map[string][]repository.StoredFolderMessage{ @@ -143,14 +145,16 @@ func TestImapSyncRemovesMailMovedIntoSkippedFolder(t *testing.T) { t.Fatalf("searched the skipped folder %d times, want once per vanished row (2)", conn.finds) } - // Next pass, nothing else changed: the row that was deleted for good is - // remembered and not searched for again. - conn.folders[0].Messages = 2 + // Next pass another message leaves, for somewhere that is not skipped. + // Only that row is searched for: the one deleted for good last pass is + // remembered, and the one already removed is not looked for twice. + conn.folders[0].Messages = 1 + conn.all = []goimap.UID{10} if err := w.Sync(t.Context()); err != nil { t.Fatalf("second Sync: %v", err) } - if conn.finds != 2 { - t.Fatalf("a settled row was searched for again (finds = %d)", conn.finds) + if conn.finds != 3 { + t.Fatalf("searched %d times over two passes, want 3: a settled row was searched for again", conn.finds) } if len(removeIDs(*events)) != 1 { t.Fatal("the second pass removed something") @@ -164,7 +168,8 @@ func TestImapSyncLeavesVanishedMailAloneWithoutSkipList(t *testing.T) { folders: []models.Mailbox{{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10, Messages: 1, Delim: "/"}}, all: []goimap.UID{8}, } - w, events := newIMAPTestMail(conn, &fixedBudget{allow: 10}, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10, Messages: 3}) + w, events := newIMAPTestMail(conn, &fixedBudget{allow: 10}, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10}) + w.rememberListing(&models.Mailbox{Name: "INBOX", Messages: 3, UIDNext: 10}) ctx := &fakeSyncContext{stored: map[string][]repository.StoredFolderMessage{ "INBOX": {{UID: 7, MessageID: "<7@fake.test>", ID: uuid.New()}}, }} @@ -180,20 +185,61 @@ func TestImapSyncLeavesVanishedMailAloneWithoutSkipList(t *testing.T) { func TestImapMovedOut(t *testing.T) { cases := []struct { - name string - before, now models.Mailbox - want bool + name string + before imapListed + now models.Mailbox + want bool }{ - {"nothing changed", models.Mailbox{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 3, UIDNext: 10}, false}, - {"one arrived", models.Mailbox{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 4, UIDNext: 11}, false}, - {"one left", models.Mailbox{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 2, UIDNext: 10}, true}, - {"one arrived and one left", models.Mailbox{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 3, UIDNext: 11}, true}, - {"no baseline from this session", models.Mailbox{Messages: 0, UIDNext: 10}, models.Mailbox{Messages: 2, UIDNext: 10}, false}, - {"cursor went backwards", models.Mailbox{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 1, UIDNext: 5}, false}, + {"nothing changed", imapListed{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 3, UIDNext: 10}, false}, + {"one arrived", imapListed{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 4, UIDNext: 11}, false}, + {"one left", imapListed{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 2, UIDNext: 10}, true}, + {"one arrived and one left", imapListed{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 3, UIDNext: 11}, true}, + {"empty before, two arrived, one left", imapListed{Messages: 0, UIDNext: 10}, models.Mailbox{Messages: 1, UIDNext: 12}, true}, + {"cursor went backwards", imapListed{Messages: 3, UIDNext: 10}, models.Mailbox{Messages: 1, UIDNext: 5}, false}, } for _, tc := range cases { - if got := imapMovedOut(&tc.before, &tc.now); got != tc.want { + if got := imapMovedOut(tc.before, &tc.now); got != tc.want { t.Errorf("%s: imapMovedOut = %v, want %v", tc.name, got, tc.want) } } } + +// The first listing of a session only baselines: a folder whose stored +// cursor came from the control plane carries no count, so nothing is +// reconciled until a second listing can be compared with the first. +func TestImapSyncBaselinesCountsOnFirstListing(t *testing.T) { + conn := &fakeImapConn{ + folders: []models.Mailbox{ + {Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10, Messages: 1, Delim: "/"}, + {Name: "Warmer", UIDValidity: 9, HighestModSeq: 100, Delim: "/"}, + }, + all: []goimap.UID{8}, + inSkipped: map[string]map[string]uint32{"Warmer": {"<7@fake.test>": 3}}, + } + budget := &skipBudget{fixedBudget: &fixedBudget{allow: 10}, skip: []string{"Warmer"}} + w, events := newIMAPTestMail(conn, budget, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10}) + ctx := &fakeSyncContext{stored: map[string][]repository.StoredFolderMessage{ + "INBOX": {{UID: 7, MessageID: "<7@fake.test>", ID: uuid.New()}}, + }} + w.SyncContext = ctx + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + if ctx.calls != 0 || len(removeIDs(*events)) != 0 { + t.Fatalf("reconciled on the first listing: lookups=%d removed=%d", ctx.calls, len(removeIDs(*events))) + } + // The second listing shows the departure against the first. + conn.folders[0].Messages = 0 + conn.all = nil + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("second Sync: %v", err) + } + if len(removeIDs(*events)) != 1 { + kinds := []models.JobEventType{} + for _, e := range *events { + kinds = append(kinds, e.eventType) + } + t.Fatalf("removed %d rows on the second listing, want 1 (lookups=%d finds=%d events=%v listed=%+v)", len(removeIDs(*events)), ctx.calls, conn.finds, kinds, w.listed) + } +} diff --git a/internal/app/worker/wmail/sync_imap_test.go b/internal/app/worker/wmail/sync_imap_test.go index 5e9744937..85e600c11 100644 --- a/internal/app/worker/wmail/sync_imap_test.go +++ b/internal/app/worker/wmail/sync_imap_test.go @@ -45,7 +45,12 @@ type fakeImapConn struct { view *imap.Selected } -func (c *fakeImapConn) Folders() ([]models.Mailbox, *errx.MailError) { return c.folders, nil } +// Folders hands out a copy, like a real listing: the pass filters the slice +// in place, and a fake that shared its backing array would lose folders +// between passes. +func (c *fakeImapConn) Folders() ([]models.Mailbox, *errx.MailError) { + return append([]models.Mailbox(nil), c.folders...), nil +} func (c *fakeImapConn) FolderOverflow() int { return c.overflow } func (c *fakeImapConn) FolderConflicts() int { return c.conflicts } diff --git a/internal/app/worker/wmail/wmail.go b/internal/app/worker/wmail/wmail.go index 670e85e09..19da6a9b1 100644 --- a/internal/app/worker/wmail/wmail.go +++ b/internal/app/worker/wmail/wmail.go @@ -102,6 +102,12 @@ type WMail struct { // synced folder and were looked for in the skipped folders without being // found, so they are not searched for again on every pass. skipChecked map[string]struct{} + // listed is the previous listing's count and UIDNEXT per folder name, + // which is what tells a pass that mail left a folder. Session-local: a + // folder is baselined on first sight and the stored cursor is not used, + // because a walked folder's cursor advances to the SELECT view and a + // server whose view lags its STATUS would read as departures every pass. + listed map[string]imapListed // unmapPending holds map entries for unpublished arrivals whose removal // failed, keyed by map key; every pass retries them before it looks. unmapPending map[string]uuid.UUID From ee9f5bf84e2188490c48b39d9d8457e00bb0ae48 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 22 Sep 2026 05:33:56 -0700 Subject: [PATCH 7/8] feat: make every skipped-folder purge drop the message map entries and parked arrivals with the rows so a message can be imported again if it moves back, persist each folder's delimiter (migration 000197) so the backend purge matches the same subfolders and case the worker skips, fail the mailbox load closed when the skip list cannot be read, mark a folder renamed into the skipped subtree as skipped, never remove a row re-fetched this pass, cap the reconciliation at 50 searches per pass and continue next pass, publish one EMAIL_DELETED after a folder purge, and resolve the account once for GET /emails/:id/sync --- internal/api/handler/email_sync.go | 7 +- internal/app/consumer/event_mailbox_delete.go | 24 ++++ internal/app/email/loader.go | 23 +++- internal/app/email/service.go | 68 ++++++---- internal/app/worker/wmail/sync_imap.go | 79 ++++++++++-- .../app/worker/wmail/sync_imap_skip_test.go | 120 ++++++++++++++++++ internal/app/worker/wmail/wmail.go | 5 + .../000197_unibox_mailboxes_delim.down.sql | 2 + .../000197_unibox_mailboxes_delim.up.sql | 8 ++ internal/repository/pg_mailbox.go | 17 +-- internal/repository/pg_unibox.go | 27 +++- 11 files changed, 322 insertions(+), 58 deletions(-) create mode 100644 internal/infrastructure/db/migrations/000197_unibox_mailboxes_delim.down.sql create mode 100644 internal/infrastructure/db/migrations/000197_unibox_mailboxes_delim.up.sql diff --git a/internal/api/handler/email_sync.go b/internal/api/handler/email_sync.go index d42e1f0b6..0accf2f60 100644 --- a/internal/api/handler/email_sync.go +++ b/internal/api/handler/email_sync.go @@ -34,12 +34,7 @@ func (h *Handler) GetEmailSync(c *gin.Context) { errx.JSON(c, errx.ErrUnauthorized) return } - state, policy, xerr := h.EmailService.GetSyncState(c.Request.Context(), orgID.String(), c.Param("id")) - if xerr != nil { - errx.JSON(c, xerr) - return - } - folders, xerr := h.EmailService.GetSyncFolders(c.Request.Context(), orgID.String(), c.Param("id")) + state, policy, folders, xerr := h.EmailService.GetSyncState(c.Request.Context(), orgID.String(), c.Param("id")) if xerr != nil { errx.JSON(c, xerr) return diff --git a/internal/app/consumer/event_mailbox_delete.go b/internal/app/consumer/event_mailbox_delete.go index 5f97f797b..8554b76d8 100644 --- a/internal/app/consumer/event_mailbox_delete.go +++ b/internal/app/consumer/event_mailbox_delete.go @@ -3,10 +3,31 @@ package jobs import ( "context" + "github.com/google/uuid" "github.com/rs/zerolog/log" + "github.com/warmbly/warmbly/internal/infrastructure/pubsub" "github.com/warmbly/warmbly/internal/models" ) +// publishFolderPurged tells open dashboards that rows left a mailbox in bulk. +// One org-scoped EMAIL_DELETED with no message id: the client drops every +// inbox list on that event whatever it names, and one event is what a purge +// of a whole folder deserves rather than one per row. +func (s *JobsService) publishFolderPurged(ctx context.Context, userID, emailID uuid.UUID) { + if s.StreamingPublisher == nil { + return + } + var orgID string + if account, err := s.EmailRepository.GetByID(ctx, emailID); err == nil && account != nil && account.OrganizationID != nil { + orgID = account.OrganizationID.String() + } + s.StreamingPublisher.PublishEmailDeleted(ctx, &pubsub.EmailInboxEvent{ + BaseEvent: pubsub.BaseEvent{UserID: userID.String()}, + OrgID: orgID, + EmailAccountID: emailID.String(), + }) +} + // HandleMailboxDelete retires a folder the last listing no longer had. // // A folder is identified by its name. UIDValidity is the fallback for an @@ -35,6 +56,9 @@ func (s *JobsService) HandleMailboxDelete(ctx context.Context, e *models.JobEven Str("folder", e.Mailbox). Int64("messages", n). Msg("folder excluded from sync: stored mail dropped") + if n > 0 { + s.publishFolderPurged(ctx, e.UserID, e.EmailID) + } } return nil } diff --git a/internal/app/email/loader.go b/internal/app/email/loader.go index b7a8b958e..763fa0ab1 100644 --- a/internal/app/email/loader.go +++ b/internal/app/email/loader.go @@ -2,6 +2,7 @@ package email import ( "context" + "fmt" "strings" "time" @@ -260,6 +261,10 @@ func (s *emailService) buildAddWorkerEmail(ctx context.Context, acc *models.Emai provider := models.InboxProvider(acc.Provider) saveToSent := acc.SaveToSent + sync, err := s.syncDataFor(ctx, acc.ID) + if err != nil { + return nil, err + } out := &models.AddWorkerEmail{ ID: acc.ID, UserID: userID, @@ -268,7 +273,7 @@ func (s *emailService) buildAddWorkerEmail(ctx context.Context, acc *models.Emai FirstName: first, LastName: last, Type: provider, - Sync: s.syncDataFor(ctx, acc.ID), + Sync: sync, // Only SMTP/IMAP acts on this; Gmail and Graph file their own copy. SaveToSent: &saveToSent, } @@ -355,7 +360,7 @@ func (s *emailService) lastHistoryFor(ctx context.Context, userID, emailID uuid. // state a previous worker left behind. Policy comes from instance settings // (compiled defaults when none are wired), so an operator's change applies at // the next load: onboarding, reassignment, or the reconciler's republish. -func (s *emailService) syncDataFor(ctx context.Context, emailID uuid.UUID) *models.AddWorkerEmailSyncData { +func (s *emailService) syncDataFor(ctx context.Context, emailID uuid.UUID) (*models.AddWorkerEmailSyncData, error) { budget := instancesettings.DefaultSync() if s.syncBudget != nil { budget = s.syncBudget.SyncBudget(ctx) @@ -368,11 +373,15 @@ func (s *emailService) syncDataFor(ctx context.Context, emailID uuid.UUID) *mode OrgDailyMessages: budget.DailyMessagesPerOrg, }, } - if skip, xerr := s.emailRepository.GetSyncSkipFolders(ctx, emailID); xerr == nil { - data.Policy.SkipFolders = skip - } else { - log.Warn().Str("email_id", emailID.String()).Msg("sync skip folders lookup failed; worker syncs every folder until the next republish") + // The skip list is part of the policy a republish replaces on the loaded + // mailbox, so a failed read cannot fall back to "skip nothing": that + // would have the worker baseline and import the excluded folders until + // the next republish. The load fails instead and the reconciler retries. + skip, xerr := s.emailRepository.GetSyncSkipFolders(ctx, emailID) + if xerr != nil { + return nil, fmt.Errorf("sync skip folders lookup: %w", xerr) } + data.Policy.SkipFolders = skip // A pool-linked mailbox is a warmup-only mirror: no history import. if s.poolLink != nil { if linked, err := s.poolLink.GetMailboxByAccount(ctx, emailID); err == nil && linked != nil { @@ -387,7 +396,7 @@ func (s *emailService) syncDataFor(ctx context.Context, emailID uuid.UUID) *mode log.Warn().Err(err).Str("email_id", emailID.String()).Msg("sync state lookup failed; worker starts fresh") } } - return data + return data, nil } // mailboxesFor is the IMAP folder state (name, UIDVALIDITY, HIGHESTMODSEQ) diff --git a/internal/app/email/service.go b/internal/app/email/service.go index 440ed0e36..14fe2a41b 100644 --- a/internal/app/email/service.go +++ b/internal/app/email/service.go @@ -2,6 +2,7 @@ package email import ( "context" + "slices" "sort" "strings" "time" @@ -133,11 +134,10 @@ type EmailService interface { // for a change outside the mailbox row (Warmbly Cloud enrollment). SyncWarmupPool(ctx context.Context, accountID uuid.UUID) // GetSyncState is the dashboard's view of a mailbox's sync: nil state when - // the worker has not reported yet. - GetSyncState(ctx context.Context, userID, emailID string) (*models.SyncState, models.SyncPolicy, *errx.Error) - // GetSyncFolders is the folders the sync has seen on the server, so a - // client can name one to skip. Empty for providers without folders. - GetSyncFolders(ctx context.Context, orgID, emailID string) ([]models.SyncFolder, *errx.Error) + // the worker has not reported yet, the policy in force, and the folders + // the sync has seen on the server so a client can name one to skip + // (empty for providers without folders). + GetSyncState(ctx context.Context, userID, emailID string) (*models.SyncState, models.SyncPolicy, []models.SyncFolder, *errx.Error) // UpdateSyncSettings replaces the mailbox's skip list, drops the mail // already stored from those folders, and re-ships the mailbox so the // worker applies it on its next pass. Returns the list as stored. @@ -366,29 +366,25 @@ func (s *emailService) publishAccountEvent(ctx context.Context, eventType pubsub // GetSyncState returns the persisted sync state and the policy currently in // force. It goes through Get so ownership is checked the same way as every // other per-mailbox read. -func (s *emailService) GetSyncState(ctx context.Context, orgID, emailID string) (*models.SyncState, models.SyncPolicy, *errx.Error) { +func (s *emailService) GetSyncState(ctx context.Context, orgID, emailID string) (*models.SyncState, models.SyncPolicy, []models.SyncFolder, *errx.Error) { acc, xerr := s.Get(ctx, orgID, emailID) if xerr != nil { - return nil, models.SyncPolicy{}, xerr + return nil, models.SyncPolicy{}, nil, xerr } - data := s.syncDataFor(ctx, acc.ID) - return data.State, data.Policy, nil + data, err := s.syncDataFor(ctx, acc.ID) + if err != nil { + log.Error().Err(err).Str("email_id", acc.ID.String()).Msg("sync state: policy lookup failed") + return nil, models.SyncPolicy{}, nil, errx.InternalError() + } + return data.State, data.Policy, s.syncFoldersFor(ctx, acc), nil } -// GetSyncFolders lists the IMAP folders the worker has reported for this +// syncFoldersFor lists the IMAP folders the worker has reported for this // mailbox, INBOX first and then by name, each with the canonical folder it // files under. Gmail and Outlook mailboxes have no folder list here. -func (s *emailService) GetSyncFolders(ctx context.Context, orgID, emailID string) ([]models.SyncFolder, *errx.Error) { - acc, xerr := s.Get(ctx, orgID, emailID) - if xerr != nil { - return nil, xerr - } +func (s *emailService) syncFoldersFor(ctx context.Context, acc *models.Email) []models.SyncFolder { out := []models.SyncFolder{} - userID, err := uuid.Parse(acc.UserID) - if acc.Provider != string(models.InboxProviderSMTPIMAP) || err != nil { - return out, nil - } - for _, box := range s.mailboxesFor(ctx, userID, acc.ID) { + for _, box := range s.imapFoldersFor(ctx, acc) { out = append(out, models.SyncFolder{Name: box.Name, Folder: imap.CanonicalFolder(box)}) } sort.SliceStable(out, func(i, j int) bool { @@ -398,14 +394,26 @@ func (s *emailService) GetSyncFolders(ctx context.Context, orgID, emailID string } return strings.ToLower(out[i].Name) < strings.ToLower(out[j].Name) }) - return out, nil + return out } -// UpdateSyncSettings stores a normalized skip list for an IMAP mailbox. The -// purge of already-stored mail is exact-name: the worker retires each -// subfolder it stops following by name on its next pass, and the re-ship -// makes that pass apply the new list within a minute rather than at the -// reconciler's next republish. +// imapFoldersFor is the saved folder listing of an IMAP mailbox, and nothing +// for any other provider. +func (s *emailService) imapFoldersFor(ctx context.Context, acc *models.Email) []models.Mailbox { + userID, err := uuid.Parse(acc.UserID) + if acc.Provider != string(models.InboxProviderSMTPIMAP) || err != nil { + return nil + } + return s.mailboxesFor(ctx, userID, acc.ID) +} + +// UpdateSyncSettings stores a normalized skip list for an IMAP mailbox and +// drops the mail already stored from every saved folder the list covers, +// decided by the same matcher the worker applies (case, subfolders), plus +// the names themselves for a folder not listed yet. The re-ship makes the +// worker's next pass apply the list within a minute rather than at the +// reconciler's next republish; a folder it retires then is purged again by +// name, which is idempotent. func (s *emailService) UpdateSyncSettings(ctx context.Context, orgID, emailID string, body *models.UpdateSyncSettings) ([]string, *errx.Error) { acc, xerr := s.Get(ctx, orgID, emailID) if xerr != nil { @@ -425,7 +433,13 @@ func (s *emailService) UpdateSyncSettings(ctx context.Context, orgID, emailID st return nil, xerr } if s.unibox != nil && len(folders) > 0 { - if n, err := s.unibox.DeleteByFolderPaths(ctx, acc.ID, folders); err != nil { + purge := append([]string(nil), folders...) + for _, box := range s.imapFoldersFor(ctx, acc) { + if imap.SkipsFolder(box, folders) && !slices.Contains(purge, box.Name) { + purge = append(purge, box.Name) + } + } + if n, err := s.unibox.DeleteByFolderPaths(ctx, acc.ID, purge); err != nil { log.Warn().Err(err).Str("email_id", acc.ID.String()).Msg("sync skip folders: purge of stored mail failed; the worker retires the folders on its next pass") } else if n > 0 { log.Info().Str("email_id", acc.ID.String()).Int64("messages", n).Msg("sync skip folders: stored mail from skipped folders dropped") diff --git a/internal/app/worker/wmail/sync_imap.go b/internal/app/worker/wmail/sync_imap.go index bcf453405..5c2a11e89 100644 --- a/internal/app/worker/wmail/sync_imap.go +++ b/internal/app/worker/wmail/sync_imap.go @@ -116,7 +116,9 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { } changed := imapFolderChanged(befBox, box, condStore) - movedOut := len(skipped) > 0 && listedBefore && imapMovedOut(prevListing, box) + // A pass cut short by the search cap left rows unexamined, so the + // next pass looks again whether or not the count moved. + movedOut := len(skipped) > 0 && (w.skipPending[box.Name] || listedBefore && imapMovedOut(prevListing, box)) fullyProcessed := true var touched map[string]struct{} var view imap.Selected @@ -162,7 +164,7 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { return err } } else if movedOut && !stats.aborted { - if err := w.imapReconcileSkipped(ctx, box, skipped, stats); err != nil { + if err := w.imapReconcileSkipped(ctx, box, skipped, touched, stats); err != nil { return err } } @@ -185,6 +187,7 @@ func (w *WMail) Sync(ctx context.Context) *errx.MailError { // Renames were already followed above, so a name missing from the listing // at this point really is a folder that is gone. var deleted []string + var gone []*models.Mailbox outer: for _, box := range w.SmtpImapData.Mailboxes { for _, f := range folders { @@ -192,7 +195,9 @@ outer: continue outer } } - + gone = append(gone, box) + } + for _, box := range gone { if err := w.onEvent(models.JobEventTypeMailboxDelete, &models.JobEventMailboxDelete{ UserID: w.UserID, EmailID: w.ID, @@ -200,7 +205,7 @@ outer: UIDValidity: box.UIDValidity, // A folder that is still on the server but now excluded takes // the mail already stored from it along. - Skipped: slices.ContainsFunc(skipped, func(s models.Mailbox) bool { return s.Name == box.Name }), + Skipped: imapRetiredIntoSkipped(box, gone, skipped), }); err != nil { return nil } @@ -211,6 +216,7 @@ outer: for _, name := range deleted { delete(w.flagScan, name) delete(w.listed, name) + delete(w.skipPending, name) // The backfill floor goes with the folder. A name is reusable, // and a floor left behind would be inherited by whatever is // created under it next. @@ -235,6 +241,32 @@ outer: return nil } +// imapRetiredIntoSkipped reports whether a folder leaving the listing is one +// the owner excluded: listed under the same name in the skipped set, or +// renamed into the skipped subtree, which the rename matcher could not see +// because skipped folders leave the listing before it runs. The rename is +// claimed on the matcher's own terms: exactly one folder gone and exactly +// one skipped folder carrying its UIDVALIDITY, so a server that stamps a +// whole tree from one creation time cannot make an unrelated deletion look +// like a move. +func imapRetiredIntoSkipped(box *models.Mailbox, gone []*models.Mailbox, skipped []models.Mailbox) bool { + if slices.ContainsFunc(skipped, func(s models.Mailbox) bool { return s.Name == box.Name }) { + return true + } + sameGone, sameSkipped := 0, 0 + for _, g := range gone { + if g.UIDValidity == box.UIDValidity { + sameGone++ + } + } + for i := range skipped { + if skipped[i].UIDValidity == box.UIDValidity { + sameSkipped++ + } + } + return sameGone == 1 && sameSkipped == 1 +} + // skipFolders is the owner's exclusion list as the policy in force carries // it; a republished ADD_EMAIL changes it between passes. func (w *WMail) skipFolders() []string { @@ -279,10 +311,19 @@ func imapMovedOut(before imapListed, now *models.Mailbox) bool { // Message-ID is found in a skipped folder. Rows checked once and found // nowhere are remembered for the session, so a folder the owner emptied by // hand does not cost a search per row on every later pass. -func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, skipped []models.Mailbox, stats *tickStats) *errx.MailError { +// +// The work is bounded per pass: at most imapSkipSearchesPerPass rows are +// looked for, and a folder with more left over is marked pending so the next +// pass continues where this one stopped. A row fetched this pass (touched) +// is live under a new UID, whatever an older copy elsewhere says. +func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, skipped []models.Mailbox, touched map[string]struct{}, stats *tickStats) *errx.MailError { if w.SyncContext == nil || len(skipped) == 0 { return nil } + if w.skipPending == nil { + w.skipPending = make(map[string]bool) + } + delete(w.skipPending, box.Name) stored, err := w.SyncContext.ListFolderMessages(ctx, w.UserID, w.ID, box.Name, box.UIDValidity) if err != nil { return w.controlPlaneError(err, stats) @@ -311,10 +352,14 @@ func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, s if w.skipChecked == nil || len(w.skipChecked) > imapSkipCheckedMax { w.skipChecked = make(map[string]struct{}) } + searches := 0 for _, m := range stored { if _, ok := live[m.UID]; ok { continue } + if _, refiled := touched[m.MessageID]; refiled { + continue + } if ctx.Err() != nil { return nil } @@ -322,6 +367,11 @@ func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, s if _, done := w.skipChecked[key]; done { continue } + if searches >= imapSkipSearchesPerPass { + w.skipPending[box.Name] = true + return nil + } + searches++ folder := w.imapFindInSkipped(ctx, skipped, m.MessageID) if folder == "" { w.skipChecked[key] = struct{}{} @@ -335,6 +385,12 @@ func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, s }); err != nil { return w.controlPlaneError(err, stats) } + // The map entry goes with the row: the sync reads a mapped + // Message-ID as already stored, and would never import the message + // again if it moved back into a synced folder. + if err := w.EmailMessageMapRepository.Del(ctx, w.UserID, w.ID, m.MessageID, m.ID); err != nil { + return w.controlPlaneError(err, stats) + } w.skipChecked[key] = struct{}{} } return nil @@ -342,7 +398,12 @@ func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, s // imapSkipCheckedMax bounds the per-session memory of rows already looked // for in the skipped folders; past it the memory starts over. -const imapSkipCheckedMax = 20_000 +const imapSkipCheckedMax = 250_000 + +// imapSkipSearchesPerPass caps the searches one folder's reconciliation +// spends in one pass, so an owner emptying a large folder by hand costs a +// bounded slice of every tick rather than one long one. +const imapSkipSearchesPerPass = 50 // imapFindInSkipped names the skipped folder holding the message, or "". // A key the sync made up for a message without a Message-ID was never on @@ -410,10 +471,10 @@ func (w *WMail) imapIncremental(ctx context.Context, box, before *models.Mailbox // Newest first: when budget is short, the freshest mail lands first. sort.Slice(uids, func(i, j int) bool { return uids[i] > uids[j] }) - // Only the drafts folder needs the fetched ids back, so nothing else pays - // for the set. + // Only the two reconciliations need the fetched ids back (drafts, and + // mail that left for a skipped folder), so nothing else pays for the set. var touched map[string]struct{} - if imapCanonicalFolder(box) == models.FolderDrafts { + if imapCanonicalFolder(box) == models.FolderDrafts || len(w.skipFolders()) > 0 { touched = make(map[string]struct{}, len(uids)) } diff --git a/internal/app/worker/wmail/sync_imap_skip_test.go b/internal/app/worker/wmail/sync_imap_skip_test.go index f18547696..3dfe5229a 100644 --- a/internal/app/worker/wmail/sync_imap_skip_test.go +++ b/internal/app/worker/wmail/sync_imap_skip_test.go @@ -2,6 +2,7 @@ package wmail import ( "context" + "fmt" "testing" goimap "github.com/emersion/go-imap/v2" @@ -243,3 +244,122 @@ func TestImapSyncBaselinesCountsOnFirstListing(t *testing.T) { t.Fatalf("removed %d rows on the second listing, want 1 (lookups=%d finds=%d events=%v listed=%+v)", len(removeIDs(*events)), ctx.calls, conn.finds, kinds, w.listed) } } + +// A synced folder renamed into the skipped subtree keeps its UIDVALIDITY, +// but the rename matcher never sees the new name because skipped folders +// leave the listing first. The retirement still carries the marker, so the +// mail stored under the old name is purged like any skipped folder's. +func TestImapSyncMarksFolderRenamedIntoSkippedSubtree(t *testing.T) { + conn := &fakeImapConn{folders: []models.Mailbox{ + {Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, Delim: "/"}, + {Name: "Warmer", UIDValidity: 9, HighestModSeq: 100, Delim: "/"}, + {Name: "Warmer/Leads", UIDValidity: 21, HighestModSeq: 50, Delim: "/"}, + {Name: "Receipts", UIDValidity: 33, HighestModSeq: 10, Delim: "/"}, + }} + budget := &skipBudget{fixedBudget: &fixedBudget{allow: 10}, skip: []string{"Warmer"}} + w, events := newIMAPTestMail(conn, budget, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100}) + w.SmtpImapData.Mailboxes = append(w.SmtpImapData.Mailboxes, + &models.Mailbox{Name: "Leads", UIDValidity: 21, HighestModSeq: 50}, + // Gone for good, and its UIDVALIDITY matches nothing skipped. + &models.Mailbox{Name: "Old", UIDValidity: 44, HighestModSeq: 5}, + ) + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + got := map[string]bool{} + for _, e := range mailboxEvents(*events, models.JobEventTypeMailboxDelete) { + del := e.body.(*models.JobEventMailboxDelete) + got[del.Mailbox] = del.Skipped + } + if skipped, ok := got["Leads"]; !ok || !skipped { + t.Fatalf("Leads retired as %v (present %v), want skipped", skipped, ok) + } + if skipped, ok := got["Old"]; !ok || skipped { + t.Fatalf("Old retired as %v (present %v), want a plain deletion", skipped, ok) + } + if len(mailboxEvents(*events, models.JobEventTypeMailboxRename)) != 0 { + t.Fatal("a rename into a skipped folder must not be followed") + } +} + +// A row whose Message-ID was fetched this pass is live under a new UID, +// whatever a copy in a skipped folder says; it is never removed. +func TestImapSyncKeepsRowRefetchedThisPass(t *testing.T) { + conn := &fakeImapConn{ + folders: []models.Mailbox{ + {Name: "INBOX", UIDValidity: 7, HighestModSeq: 200, UIDNext: 12, Messages: 1, Delim: "/"}, + {Name: "Warmer", UIDValidity: 9, HighestModSeq: 100, Delim: "/"}, + }, + changed: []goimap.UID{11}, + all: []goimap.UID{11}, + inSkipped: map[string]map[string]uint32{"Warmer": {"<11@fake.test>": 3}}, + } + budget := &skipBudget{fixedBudget: &fixedBudget{allow: 10}, skip: []string{"Warmer"}} + w, events := newIMAPTestMail(conn, budget, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10}) + w.rememberListing(&models.Mailbox{Name: "INBOX", Messages: 1, UIDNext: 10}) + w.EmailMessageMapRepository = knownMessageMap{id: uuid.New().String()} + // The stored row is the same message under its old UID: re-appended + // this pass as UID 11 (a warmup engagement leg moved it out and back). + w.SyncContext = &fakeSyncContext{stored: map[string][]repository.StoredFolderMessage{ + "INBOX": {{UID: 5, MessageID: "<11@fake.test>", ID: uuid.New()}}, + }} + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + if n := len(removeIDs(*events)); n != 0 { + t.Fatalf("removed %d rows, want none: the message is live under a new UID", n) + } + if conn.finds != 0 { + t.Fatalf("searched %d times for a row fetched this pass", conn.finds) + } +} + +// The reconciliation spends at most imapSkipSearchesPerPass searches on one +// folder per pass and continues on the next, so a folder emptied by hand is +// examined in slices rather than in one long tick. +func TestImapSyncCapsSkippedSearchesPerPass(t *testing.T) { + conn := &fakeImapConn{ + folders: []models.Mailbox{ + {Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 500, Messages: 0, Delim: "/"}, + {Name: "Warmer", UIDValidity: 9, HighestModSeq: 100, Delim: "/"}, + }, + inSkipped: map[string]map[string]uint32{"Warmer": {}}, + } + budget := &skipBudget{fixedBudget: &fixedBudget{allow: 10}, skip: []string{"Warmer"}} + w, events := newIMAPTestMail(conn, budget, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 500}) + w.rememberListing(&models.Mailbox{Name: "INBOX", Messages: 120, UIDNext: 500}) + w.EmailMessageMapRepository = knownMessageMap{id: uuid.New().String()} + stored := make([]repository.StoredFolderMessage, 0, 120) + for i := 1; i <= 120; i++ { + id := fmt.Sprintf("<%d@fake.test>", i) + stored = append(stored, repository.StoredFolderMessage{UID: uint32(i), MessageID: id, ID: uuid.New()}) + } + // The last one is in the skipped folder; it is reached on the third pass. + conn.inSkipped["Warmer"]["<120@fake.test>"] = 9 + ctx := &fakeSyncContext{stored: map[string][]repository.StoredFolderMessage{"INBOX": stored}} + w.SyncContext = ctx + + for pass, wantFinds := range []int{imapSkipSearchesPerPass, 2 * imapSkipSearchesPerPass, 120} { + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("pass %d: %v", pass+1, err) + } + if conn.finds != wantFinds { + t.Fatalf("pass %d: %d searches so far, want %d", pass+1, conn.finds, wantFinds) + } + } + if n := len(removeIDs(*events)); n != 1 { + t.Fatalf("removed %d rows over three passes, want the one found in Warmer", n) + } + if w.skipPending["INBOX"] { + t.Fatal("the folder is still marked pending after every row was examined") + } + // A fourth pass with nothing new does not look again. + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("fourth pass: %v", err) + } + if conn.finds != 120 { + t.Fatalf("a settled folder was searched again (finds = %d)", conn.finds) + } +} diff --git a/internal/app/worker/wmail/wmail.go b/internal/app/worker/wmail/wmail.go index 19da6a9b1..3e6e594c1 100644 --- a/internal/app/worker/wmail/wmail.go +++ b/internal/app/worker/wmail/wmail.go @@ -107,7 +107,12 @@ type WMail struct { // folder is baselined on first sight and the stored cursor is not used, // because a walked folder's cursor advances to the SELECT view and a // server whose view lags its STATUS would read as departures every pass. + // A move that lands while the worker is down is therefore not seen; the + // row it leaves is retired by the next departure from that folder. listed map[string]imapListed + // skipPending marks folders whose skipped-folder reconciliation hit the + // per-pass search cap, so the next pass continues it. + skipPending map[string]bool // unmapPending holds map entries for unpublished arrivals whose removal // failed, keyed by map key; every pass retries them before it looks. unmapPending map[string]uuid.UUID diff --git a/internal/infrastructure/db/migrations/000197_unibox_mailboxes_delim.down.sql b/internal/infrastructure/db/migrations/000197_unibox_mailboxes_delim.down.sql new file mode 100644 index 000000000..e8e53ed4f --- /dev/null +++ b/internal/infrastructure/db/migrations/000197_unibox_mailboxes_delim.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE public.unibox_mailboxes + DROP COLUMN IF EXISTS delim; diff --git a/internal/infrastructure/db/migrations/000197_unibox_mailboxes_delim.up.sql b/internal/infrastructure/db/migrations/000197_unibox_mailboxes_delim.up.sql new file mode 100644 index 000000000..bd9da6ea6 --- /dev/null +++ b/internal/infrastructure/db/migrations/000197_unibox_mailboxes_delim.up.sql @@ -0,0 +1,8 @@ +-- The hierarchy delimiter a server reported for each folder is kept with the +-- folder, so the control plane can tell a subfolder from a sibling whose +-- name merely starts the same way. The worker matches a skipped folder's +-- subfolders on it; the purge that follows a folder being excluded from sync +-- has to reach the same set of stored rows. Empty when the server reported +-- none, where a folder has no subfolders to speak of. +ALTER TABLE public.unibox_mailboxes + ADD COLUMN delim text NOT NULL DEFAULT ''; diff --git a/internal/repository/pg_mailbox.go b/internal/repository/pg_mailbox.go index fd16bd4b8..c7a58f6fd 100644 --- a/internal/repository/pg_mailbox.go +++ b/internal/repository/pg_mailbox.go @@ -41,19 +41,20 @@ func (r *mailboxRepository) CreateEntry(ctx context.Context, userId, emailId uui mb.UpdatedAt = time.Now() query := ` - INSERT INTO unibox_mailboxes (email_id, uid_validity, mailbox, attributes, highestmodseq, uid_next, updated_at) - VALUES ($1, $2, $3, $4, $5, $6, $7) + INSERT INTO unibox_mailboxes (email_id, uid_validity, mailbox, attributes, highestmodseq, uid_next, updated_at, delim) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) ON CONFLICT (email_id, mailbox) DO UPDATE SET uid_validity = EXCLUDED.uid_validity, attributes = EXCLUDED.attributes, highestmodseq = EXCLUDED.highestmodseq, uid_next = EXCLUDED.uid_next, - updated_at = EXCLUDED.updated_at + updated_at = EXCLUDED.updated_at, + delim = EXCLUDED.delim ` // attributes is NOT NULL; a nil slice binds as SQL NULL. See textArray. _, err := r.db.Exec(ctx, query, - emailId, mb.UIDValidity, mb.Name, textArray(mb.Attrs), mb.HighestModSeq, mb.UIDNext, mb.UpdatedAt, + emailId, mb.UIDValidity, mb.Name, textArray(mb.Attrs), mb.HighestModSeq, mb.UIDNext, mb.UpdatedAt, mb.Delim, ) // The mailbox was deleted between the worker listing its folders and this // write landing. There is no parent to hang a folder off and never will be @@ -68,14 +69,14 @@ func (r *mailboxRepository) CreateEntry(ctx context.Context, userId, emailId uui func (r *mailboxRepository) GetMailbox(ctx context.Context, userId, emailId uuid.UUID, name string) (*models.Mailbox, error) { query := ` - SELECT mailbox, attributes, uid_validity, highestmodseq, uid_next, updated_at + SELECT mailbox, attributes, uid_validity, highestmodseq, uid_next, updated_at, delim FROM unibox_mailboxes WHERE email_id = $1 AND mailbox = $2 ` var mb models.Mailbox err := r.db.QueryRow(ctx, query, emailId, name).Scan( - &mb.Name, &mb.Attrs, &mb.UIDValidity, &mb.HighestModSeq, &mb.UIDNext, &mb.UpdatedAt, + &mb.Name, &mb.Attrs, &mb.UIDValidity, &mb.HighestModSeq, &mb.UIDNext, &mb.UpdatedAt, &mb.Delim, ) if err != nil { if err == pgx.ErrNoRows { @@ -89,7 +90,7 @@ func (r *mailboxRepository) GetMailbox(ctx context.Context, userId, emailId uuid func (r *mailboxRepository) ListMailboxes(ctx context.Context, userId, emailId uuid.UUID) ([]models.Mailbox, error) { query := ` - SELECT mailbox, attributes, uid_validity, highestmodseq, uid_next, updated_at + SELECT mailbox, attributes, uid_validity, highestmodseq, uid_next, updated_at, delim FROM unibox_mailboxes WHERE email_id = $1 ` @@ -103,7 +104,7 @@ func (r *mailboxRepository) ListMailboxes(ctx context.Context, userId, emailId u var mailboxes []models.Mailbox for rows.Next() { var mb models.Mailbox - if err := rows.Scan(&mb.Name, &mb.Attrs, &mb.UIDValidity, &mb.HighestModSeq, &mb.UIDNext, &mb.UpdatedAt); err != nil { + if err := rows.Scan(&mb.Name, &mb.Attrs, &mb.UIDValidity, &mb.HighestModSeq, &mb.UIDNext, &mb.UpdatedAt, &mb.Delim); err != nil { return nil, err } mailboxes = append(mailboxes, mb) diff --git a/internal/repository/pg_unibox.go b/internal/repository/pg_unibox.go index a42c2043c..34a45fbf2 100644 --- a/internal/repository/pg_unibox.go +++ b/internal/repository/pg_unibox.go @@ -924,14 +924,39 @@ func (r *uniboxRepository) Delete(ctx context.Context, userID, id uuid.UUID) err // DeleteByFolderPaths removes the mirror rows for whole source folders. The // mail stays where it is at the provider; only the platform's copy goes. +// The message map entries go with the rows: the sync reads a mapped +// Message-ID as already stored, so an entry left behind would keep the +// message from ever being imported again if it moved back into a synced +// folder. An arrival still parked on warmup verification is dropped too, or +// it would surface into a folder nobody follows. func (r *uniboxRepository) DeleteByFolderPaths(ctx context.Context, emailID uuid.UUID, folderPaths []string) (int64, error) { if len(folderPaths) == 0 { return 0, nil } - tag, err := r.db.Exec(ctx, `DELETE FROM unibox_emails WHERE email_id = $1 AND folder_path = ANY($2)`, emailID, folderPaths) + tx, err := r.db.Begin(ctx) if err != nil { return 0, err } + defer func() { _ = tx.Rollback(ctx) }() + if _, err := tx.Exec(ctx, ` + DELETE FROM email_message_map m + USING unibox_emails u + WHERE u.email_id = $1 AND u.folder_path = ANY($2) + AND m.email_id = u.email_id AND m.message_id = u.message_id`, emailID, folderPaths); err != nil { + return 0, err + } + if _, err := tx.Exec(ctx, ` + DELETE FROM unibox_pending_emails + WHERE email_account_id = $1 AND payload->'message'->>'folder_path' = ANY($2)`, emailID, folderPaths); err != nil { + return 0, err + } + tag, err := tx.Exec(ctx, `DELETE FROM unibox_emails WHERE email_id = $1 AND folder_path = ANY($2)`, emailID, folderPaths) + if err != nil { + return 0, err + } + if err := tx.Commit(ctx); err != nil { + return 0, err + } return tag.RowsAffected(), nil } From 1520da2fa88ff7d5e16d43ba90e8cf2527754166 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Tue, 22 Sep 2026 05:45:38 -0700 Subject: [PATCH 8/8] feat: look up moved mail against each skipped folder with one SELECT per folder and cap the reconciliation by searches rather than rows, never settle a row whose lookup failed, refuse a skip name that is a saved folder the matcher keeps and purge only what it will stop following, drop the map entries of parked arrivals too, ignore UIDVALIDITY 0 when matching a rename into the skipped subtree, carry the listing memo and pending mark across a rename, ask before ticking a folder in the drawer since imported mail is removed, share one inbox-deleted publisher between the consumer's removal paths, and say in the CLI help that an emptied list does not restore removed mail --- cmd/cli/specs.go | 5 +- internal/app/consumer/event_mailbox_delete.go | 12 +- internal/app/consumer/event_remove_email.go | 16 +-- internal/app/email/service.go | 34 ++++- internal/app/worker/wmail/imap_conn.go | 3 + internal/app/worker/wmail/sync_imap.go | 133 +++++++++++------- .../app/worker/wmail/sync_imap_skip_test.go | 52 ++++++- internal/app/worker/wmail/sync_imap_test.go | 1 + internal/app/worker/wmail/wmail.go | 9 +- .../client/smtpimap/imap/warmup_actions.go | 43 ++++++ internal/repository/pg_unibox.go | 7 + .../components/app/emails/SyncStatusCard.tsx | 20 ++- 12 files changed, 242 insertions(+), 93 deletions(-) diff --git a/cmd/cli/specs.go b/cmd/cli/specs.go index 60bd3be22..e57aaeaf6 100644 --- a/cmd/cli/specs.go +++ b/cmd/cli/specs.go @@ -671,8 +671,9 @@ so a removed alias stops being used instead of failing every send.`, server lists them (see "mailbox sync" for the names). Each also covers its subfolders. Mail already imported from a folder is removed from Warmbly when it is skipped; the mail itself stays in the mailbox. Inbox, sent, -drafts, spam, trash and archive cannot be skipped. To sync everything -again, send an empty list with --input.`, +drafts, spam, trash and archive cannot be skipped. Sending an empty list +with --input follows every folder again from then on; the mail removed +while a folder was skipped is not brought back.`, Example: " $ warmbly mailbox skip-folders MAILBOX_ID --folder Warmer\n $ warmbly mailbox skip-folders MAILBOX_ID --folder Warmer --folder \"Clients/Acme\"\n $ warmbly mailbox skip-folders MAILBOX_ID --input '{\"skip_folders\": []}'", Method: http.MethodPut, Path: "/emails/{id}/sync", Body: bodyRequired, Args: []argSpec{{Name: "id", Help: "The mailbox's id"}}, diff --git a/internal/app/consumer/event_mailbox_delete.go b/internal/app/consumer/event_mailbox_delete.go index 8554b76d8..31687323d 100644 --- a/internal/app/consumer/event_mailbox_delete.go +++ b/internal/app/consumer/event_mailbox_delete.go @@ -9,11 +9,10 @@ import ( "github.com/warmbly/warmbly/internal/models" ) -// publishFolderPurged tells open dashboards that rows left a mailbox in bulk. -// One org-scoped EMAIL_DELETED with no message id: the client drops every -// inbox list on that event whatever it names, and one event is what a purge -// of a whole folder deserves rather than one per row. -func (s *JobsService) publishFolderPurged(ctx context.Context, userID, emailID uuid.UUID) { +// publishInboxDeleted tells open dashboards a row is gone, org-scoped so +// every teammate's inbox drops it live. An empty message id means a whole +// folder went: the client drops every inbox list on the event either way. +func (s *JobsService) publishInboxDeleted(ctx context.Context, userID, emailID uuid.UUID, messageID string) { if s.StreamingPublisher == nil { return } @@ -25,6 +24,7 @@ func (s *JobsService) publishFolderPurged(ctx context.Context, userID, emailID u BaseEvent: pubsub.BaseEvent{UserID: userID.String()}, OrgID: orgID, EmailAccountID: emailID.String(), + MessageID: messageID, }) } @@ -57,7 +57,7 @@ func (s *JobsService) HandleMailboxDelete(ctx context.Context, e *models.JobEven Int64("messages", n). Msg("folder excluded from sync: stored mail dropped") if n > 0 { - s.publishFolderPurged(ctx, e.UserID, e.EmailID) + s.publishInboxDeleted(ctx, e.UserID, e.EmailID, "") } } return nil diff --git a/internal/app/consumer/event_remove_email.go b/internal/app/consumer/event_remove_email.go index faa4549ba..148a04396 100644 --- a/internal/app/consumer/event_remove_email.go +++ b/internal/app/consumer/event_remove_email.go @@ -7,7 +7,6 @@ import ( "github.com/rs/zerolog/log" "github.com/warmbly/warmbly/internal/config" - "github.com/warmbly/warmbly/internal/infrastructure/pubsub" "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/repository" ) @@ -52,20 +51,7 @@ func (s *JobsService) HandleRemoveEmail(ctx context.Context, e *models.JobEventR _ = s.UniboxRepository.Delete(ctx, e.UserID, e.ID) } - // Tell open dashboards the row is gone (org-scoped so every teammate's - // unibox drops it live, not just the mailbox owner's). - if s.StreamingPublisher != nil { - var orgID string - if account, err := s.EmailRepository.GetByID(ctx, e.EmailID); err == nil && account != nil && account.OrganizationID != nil { - orgID = account.OrganizationID.String() - } - s.StreamingPublisher.PublishEmailDeleted(ctx, &pubsub.EmailInboxEvent{ - BaseEvent: pubsub.BaseEvent{UserID: e.UserID.String()}, - OrgID: orgID, - EmailAccountID: e.EmailID.String(), - MessageID: e.ID.String(), - }) - } + s.publishInboxDeleted(ctx, e.UserID, e.EmailID, e.ID.String()) return nil } diff --git a/internal/app/email/service.go b/internal/app/email/service.go index 14fe2a41b..9ee1dba79 100644 --- a/internal/app/email/service.go +++ b/internal/app/email/service.go @@ -2,6 +2,7 @@ package email import ( "context" + "fmt" "slices" "sort" "strings" @@ -429,16 +430,35 @@ func (s *emailService) UpdateSyncSettings(ctx context.Context, orgID, emailID st if xerr != nil { return nil, xerr } + // The purge reaches exactly what the worker will stop following: every + // saved folder the matcher skips, plus a name no saved folder answers to + // (a folder not listed yet). A name that IS a saved folder the matcher + // refuses, by attribute, is refused here too rather than purged. + saved := s.imapFoldersFor(ctx, acc) + var purge []string + for _, name := range folders { + listed := false + for _, box := range saved { + if strings.EqualFold(box.Name, name) { + listed = true + if !imap.SkipsFolder(box, folders) { + return nil, errx.NewWithIdentifier(errx.BadRequest, "invalid_sync_folder", fmt.Sprintf("folder %q is a folder the sync always follows", box.Name)) + } + } + } + if !listed { + purge = append(purge, name) + } + } + for _, box := range saved { + if imap.SkipsFolder(box, folders) && !slices.Contains(purge, box.Name) { + purge = append(purge, box.Name) + } + } if xerr := s.emailRepository.SetSyncSkipFolders(ctx, orgID, emailID, folders); xerr != nil { return nil, xerr } - if s.unibox != nil && len(folders) > 0 { - purge := append([]string(nil), folders...) - for _, box := range s.imapFoldersFor(ctx, acc) { - if imap.SkipsFolder(box, folders) && !slices.Contains(purge, box.Name) { - purge = append(purge, box.Name) - } - } + if s.unibox != nil && len(purge) > 0 { if n, err := s.unibox.DeleteByFolderPaths(ctx, acc.ID, purge); err != nil { log.Warn().Err(err).Str("email_id", acc.ID.String()).Msg("sync skip folders: purge of stored mail failed; the worker retires the folders on its next pass") } else if n > 0 { diff --git a/internal/app/worker/wmail/imap_conn.go b/internal/app/worker/wmail/imap_conn.go index 49c00ce2c..55edd6007 100644 --- a/internal/app/worker/wmail/imap_conn.go +++ b/internal/app/worker/wmail/imap_conn.go @@ -58,6 +58,9 @@ type ImapConn interface { // FindUIDByMessageID relocates a warmup message whose UID went void when an // earlier engagement leg moved it. FindUIDByMessageID(ctx context.Context, mailboxName, rfcMessageID string) (uint32, error) + // FindUIDsByMessageIDs answers which of many ids one folder holds, with + // one SELECT: the skipped-folder reconciliation's lookup. + FindUIDsByMessageIDs(ctx context.Context, mailboxName string, rfcMessageIDs []string) (map[string]uint32, error) // DeleteUID removes one message, the retention window's deletion: an // expunge scoped to the UID, or a move into trashName where the server // cannot scope one. diff --git a/internal/app/worker/wmail/sync_imap.go b/internal/app/worker/wmail/sync_imap.go index 5c2a11e89..c775013ff 100644 --- a/internal/app/worker/wmail/sync_imap.go +++ b/internal/app/worker/wmail/sync_imap.go @@ -15,6 +15,7 @@ import ( "github.com/warmbly/warmbly/internal/config" "github.com/warmbly/warmbly/internal/errx" "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/repository" ) // Sync is one IMAP pass: follow every folder's CONDSTORE mod-sequence for @@ -253,6 +254,11 @@ func imapRetiredIntoSkipped(box *models.Mailbox, gone []*models.Mailbox, skipped if slices.ContainsFunc(skipped, func(s models.Mailbox) bool { return s.Name == box.Name }) { return true } + // A folder without a UIDVALIDITY cannot be matched on it, as in + // imapFollowRenames. + if box.UIDValidity == 0 { + return false + } sameGone, sameSkipped := 0, 0 for _, g := range gone { if g.UIDValidity == box.UIDValidity { @@ -305,17 +311,12 @@ func imapMovedOut(before imapListed, now *models.Mailbox) bool { } // imapReconcileSkipped retires the platform's rows for mail that left this -// folder for one the owner excluded from sync. Nothing else that leaves a -// folder is touched: a message can go somewhere the sync does not follow -// (Gmail's All Mail) and still be wanted, so a row goes only when its -// Message-ID is found in a skipped folder. Rows checked once and found -// nowhere are remembered for the session, so a folder the owner emptied by -// hand does not cost a search per row on every later pass. -// -// The work is bounded per pass: at most imapSkipSearchesPerPass rows are -// looked for, and a folder with more left over is marked pending so the next -// pass continues where this one stopped. A row fetched this pass (touched) -// is live under a new UID, whatever an older copy elsewhere says. +// folder for one the owner excluded from sync. A row goes only when its +// Message-ID is found in a skipped folder: mail can leave for somewhere the +// sync does not follow (Gmail's All Mail) and still be wanted. Bounded per +// pass by imapSkipSearchesPerPass searches; what is left is continued next +// pass. A row looked for and found nowhere is remembered for the session; a +// lookup that failed is not, so the row is looked for again. func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, skipped []models.Mailbox, touched map[string]struct{}, stats *tickStats) *errx.MailError { if w.SyncContext == nil || len(skipped) == 0 { return nil @@ -336,8 +337,7 @@ func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, s if serr != nil { return serr } - // The same guard as the drafts reconciliation: UIDs only mean anything - // inside one generation. + // UIDs only mean anything inside one generation. if gen != box.UIDValidity { return nil } @@ -352,7 +352,11 @@ func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, s if w.skipChecked == nil || len(w.skipChecked) > imapSkipCheckedMax { w.skipChecked = make(map[string]struct{}) } - searches := 0 + + // The rows worth a lookup: gone from the folder, not re-fetched this + // pass under a new UID, not settled earlier this session, and with a + // Message-ID that was ever on the wire. + var candidates []repository.StoredFolderMessage for _, m := range stored { if _, ok := live[m.UID]; ok { continue @@ -360,21 +364,58 @@ func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, s if _, refiled := touched[m.MessageID]; refiled { continue } - if ctx.Err() != nil { - return nil - } - key := fmt.Sprintf("%s\x00%d\x00%d", box.Name, box.UIDValidity, m.UID) - if _, done := w.skipChecked[key]; done { + if _, done := w.skipChecked[w.skipKey(box, m.UID)]; done { continue } - if searches >= imapSkipSearchesPerPass { + if m.MessageID == "" || strings.HasPrefix(m.MessageID, "no-msgid/") { + w.skipChecked[w.skipKey(box, m.UID)] = struct{}{} + continue + } + candidates = append(candidates, m) + } + if len(candidates) == 0 { + return nil + } + // One SEARCH per row per skipped folder is the cost; the cap is on that. + batch := max(1, imapSkipSearchesPerPass/len(skipped)) + if len(candidates) > batch { + w.skipPending[box.Name] = true + candidates = candidates[:batch] + } + ids := make([]string, 0, len(candidates)) + for _, m := range candidates { + ids = append(ids, m.MessageID) + } + + foundIn := make(map[string]string, len(ids)) + complete := true + for i := range skipped { + if ctx.Err() != nil { w.skipPending[box.Name] = true return nil } - searches++ - folder := w.imapFindInSkipped(ctx, skipped, m.MessageID) - if folder == "" { - w.skipChecked[key] = struct{}{} + found, ferr := client.FindUIDsByMessageIDs(ctx, skipped[i].Name, ids) + if ferr != nil { + log.Debug().Err(ferr).Str("email_id", w.ID.String()).Str("folder", skipped[i].Name).Msg("sync: search in skipped folder failed") + complete = false + continue + } + for id := range found { + if _, ok := foundIn[id]; !ok { + foundIn[id] = skipped[i].Name + } + } + } + + for _, m := range candidates { + folder, ok := foundIn[m.MessageID] + if !ok { + // Settled only when every skipped folder answered. + if complete { + w.skipChecked[w.skipKey(box, m.UID)] = struct{}{} + } else { + w.skipPending[box.Name] = true + } continue } if err := w.onEvent(models.JobEventTypeRemoveEmail, &models.JobEventRemoveEmail{ @@ -385,46 +426,30 @@ func (w *WMail) imapReconcileSkipped(ctx context.Context, box *models.Mailbox, s }); err != nil { return w.controlPlaneError(err, stats) } - // The map entry goes with the row: the sync reads a mapped - // Message-ID as already stored, and would never import the message - // again if it moved back into a synced folder. + // The map entry goes with the row, or the message could never be + // imported again after moving back into a synced folder. if err := w.EmailMessageMapRepository.Del(ctx, w.UserID, w.ID, m.MessageID, m.ID); err != nil { return w.controlPlaneError(err, stats) } - w.skipChecked[key] = struct{}{} + w.skipChecked[w.skipKey(box, m.UID)] = struct{}{} } return nil } +// skipKey identifies one stored row for the session memory: folder, +// generation and UID. +func (w *WMail) skipKey(box *models.Mailbox, uid uint32) string { + return fmt.Sprintf("%s\x00%d\x00%d", box.Name, box.UIDValidity, uid) +} + // imapSkipCheckedMax bounds the per-session memory of rows already looked // for in the skipped folders; past it the memory starts over. const imapSkipCheckedMax = 250_000 // imapSkipSearchesPerPass caps the searches one folder's reconciliation -// spends in one pass, so an owner emptying a large folder by hand costs a -// bounded slice of every tick rather than one long one. +// spends in one pass, across every skipped folder. const imapSkipSearchesPerPass = 50 -// imapFindInSkipped names the skipped folder holding the message, or "". -// A key the sync made up for a message without a Message-ID was never on -// the wire, so there is nothing to search for. -func (w *WMail) imapFindInSkipped(ctx context.Context, skipped []models.Mailbox, messageID string) string { - if messageID == "" || strings.HasPrefix(messageID, "no-msgid/") { - return "" - } - for i := range skipped { - uid, err := w.SmtpImapData.ImapClient.FindUIDByMessageID(ctx, skipped[i].Name, messageID) - if err != nil { - log.Debug().Err(err).Str("email_id", w.ID.String()).Str("folder", skipped[i].Name).Msg("sync: search in skipped folder failed") - continue - } - if uid != 0 { - return skipped[i].Name - } - } - return "" -} - // imapFolderChanged reports whether a folder has anything new since the // cursor we hold for it. With CONDSTORE the mod-sequence answers for new mail // AND flag changes; without it only arrivals are visible here, and flag @@ -985,6 +1010,14 @@ func (w *WMail) imapFollowRenames(folders []models.Mailbox) error { delete(w.flagScan, from.Name) w.flagScan[to[0].Name] = scan } + if l, ok := w.listed[from.Name]; ok { + delete(w.listed, from.Name) + w.listed[to[0].Name] = l + } + if w.skipPending[from.Name] { + delete(w.skipPending, from.Name) + w.skipPending[to[0].Name] = true + } w.tracker.renameFolder(from.Name, to[0].Name) from.Name = to[0].Name } diff --git a/internal/app/worker/wmail/sync_imap_skip_test.go b/internal/app/worker/wmail/sync_imap_skip_test.go index 3dfe5229a..1ec1bc7a7 100644 --- a/internal/app/worker/wmail/sync_imap_skip_test.go +++ b/internal/app/worker/wmail/sync_imap_skip_test.go @@ -11,9 +11,21 @@ import ( "github.com/warmbly/warmbly/internal/repository" ) -func (c *fakeImapConn) FindUIDByMessageID(_ context.Context, folder, messageID string) (uint32, error) { - c.finds++ - return c.inSkipped[folder][messageID], nil +// FindUIDsByMessageIDs counts every id it was asked about, so finds is the +// number of searches the reconciliation spent. failFinds makes it fail the +// way a dropped connection does. +func (c *fakeImapConn) FindUIDsByMessageIDs(_ context.Context, folder string, ids []string) (map[string]uint32, error) { + c.finds += len(ids) + if c.failFinds { + return nil, fmt.Errorf("connection dropped") + } + out := map[string]uint32{} + for _, id := range ids { + if uid := c.inSkipped[folder][id]; uid != 0 { + out[id] = uid + } + } + return out, nil } // skipBudget is fixedBudget with an owner's skip list in the policy. @@ -363,3 +375,37 @@ func TestImapSyncCapsSkippedSearchesPerPass(t *testing.T) { t.Fatalf("a settled folder was searched again (finds = %d)", conn.finds) } } + +// A lookup that failed settles nothing: the row is looked for again on the +// next pass, and retired once the skipped folder answers. +func TestImapSyncRetriesRowsWhoseLookupFailed(t *testing.T) { + conn := &fakeImapConn{ + folders: []models.Mailbox{ + {Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10, Messages: 0, Delim: "/"}, + {Name: "Warmer", UIDValidity: 9, HighestModSeq: 100, Delim: "/"}, + }, + inSkipped: map[string]map[string]uint32{"Warmer": {"<7@fake.test>": 3}}, + failFinds: true, + } + budget := &skipBudget{fixedBudget: &fixedBudget{allow: 10}, skip: []string{"Warmer"}} + w, events := newIMAPTestMail(conn, budget, &models.Mailbox{Name: "INBOX", UIDValidity: 7, HighestModSeq: 100, UIDNext: 10}) + w.rememberListing(&models.Mailbox{Name: "INBOX", Messages: 1, UIDNext: 10}) + w.EmailMessageMapRepository = knownMessageMap{id: uuid.New().String()} + w.SyncContext = &fakeSyncContext{stored: map[string][]repository.StoredFolderMessage{ + "INBOX": {{UID: 7, MessageID: "<7@fake.test>", ID: uuid.New()}}, + }} + + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("Sync: %v", err) + } + if len(removeIDs(*events)) != 0 || !w.skipPending["INBOX"] { + t.Fatalf("a failed lookup settled the row: removed=%d pending=%v", len(removeIDs(*events)), w.skipPending["INBOX"]) + } + conn.failFinds = false + if err := w.Sync(t.Context()); err != nil { + t.Fatalf("second Sync: %v", err) + } + if len(removeIDs(*events)) != 1 { + t.Fatalf("removed %d rows once the lookup worked, want 1", len(removeIDs(*events))) + } +} diff --git a/internal/app/worker/wmail/sync_imap_test.go b/internal/app/worker/wmail/sync_imap_test.go index 85e600c11..5e660b626 100644 --- a/internal/app/worker/wmail/sync_imap_test.go +++ b/internal/app/worker/wmail/sync_imap_test.go @@ -40,6 +40,7 @@ type fakeImapConn struct { // finds counts how often it was asked. inSkipped map[string]map[string]uint32 finds int + failFinds bool // view, when set, is the cursors SELECT reports in place of the listing's: // a server whose selected view lags or leads its STATUS. view *imap.Selected diff --git a/internal/app/worker/wmail/wmail.go b/internal/app/worker/wmail/wmail.go index 3e6e594c1..e8f2eb62e 100644 --- a/internal/app/worker/wmail/wmail.go +++ b/internal/app/worker/wmail/wmail.go @@ -102,13 +102,8 @@ type WMail struct { // synced folder and were looked for in the skipped folders without being // found, so they are not searched for again on every pass. skipChecked map[string]struct{} - // listed is the previous listing's count and UIDNEXT per folder name, - // which is what tells a pass that mail left a folder. Session-local: a - // folder is baselined on first sight and the stored cursor is not used, - // because a walked folder's cursor advances to the SELECT view and a - // server whose view lags its STATUS would read as departures every pass. - // A move that lands while the worker is down is therefore not seen; the - // row it leaves is retired by the next departure from that folder. + // listed is the previous listing's count and UIDNEXT per folder name: + // session-local, since the stored cursor advances to the SELECT view. listed map[string]imapListed // skipPending marks folders whose skipped-folder reconciliation hit the // per-pass search cap, so the next pass continues it. diff --git a/internal/client/smtpimap/imap/warmup_actions.go b/internal/client/smtpimap/imap/warmup_actions.go index afd85a5aa..5f2647aeb 100644 --- a/internal/client/smtpimap/imap/warmup_actions.go +++ b/internal/client/smtpimap/imap/warmup_actions.go @@ -56,6 +56,49 @@ func (c *Client) FindUIDByMessageID(ctx context.Context, mailboxName, rfcMessage return uint32(uids[len(uids)-1]), nil } +// FindUIDsByMessageIDs is FindUIDByMessageID for many ids against one +// folder: one SELECT, then one SEARCH per id. It answers only the ids it +// found. A folder that does not exist answers nothing and is not an error. +func (c *Client) FindUIDsByMessageIDs(ctx context.Context, mailboxName string, rfcMessageIDs []string) (map[string]uint32, error) { + found := make(map[string]uint32, len(rfcMessageIDs)) + if mailboxName == "" || len(rfcMessageIDs) == 0 { + return found, nil + } + + c.mu.Lock() + defer c.mu.Unlock() + + if merr := c.ensureConnected(); merr != nil { + return nil, merr + } + c.lifecycle.RLock() + defer c.lifecycle.RUnlock() + defer c.begin()() + name := c.qualifyMailboxLocked(mailboxName) + if _, err := c.selectMailbox(name, nil); err != nil { + return found, nil + } + for _, raw := range rfcMessageIDs { + if ctx.Err() != nil { + return nil, ctx.Err() + } + id := strings.TrimSpace(raw) + if id == "" { + continue + } + data, err := c.client.UIDSearch(&imap.SearchCriteria{ + Header: []imap.SearchCriteriaHeaderField{{Key: "Message-Id", Value: "<" + strings.Trim(id, "<>") + ">"}}, + }, nil).Wait() + if err != nil { + return nil, fmt.Errorf("search %q for message id: %w", name, err) + } + if uids := data.AllUIDs(); len(uids) > 0 { + found[raw] = uint32(uids[len(uids)-1]) + } + } + return found, nil +} + // MarkAsRead sets the \Seen flag on the given UID in mailboxName. func (c *Client) MarkAsRead(ctx context.Context, mailboxName string, uid uint32) error { c.mu.Lock() diff --git a/internal/repository/pg_unibox.go b/internal/repository/pg_unibox.go index 34a45fbf2..9d666feb2 100644 --- a/internal/repository/pg_unibox.go +++ b/internal/repository/pg_unibox.go @@ -945,6 +945,13 @@ func (r *uniboxRepository) DeleteByFolderPaths(ctx context.Context, emailID uuid AND m.email_id = u.email_id AND m.message_id = u.message_id`, emailID, folderPaths); err != nil { return 0, err } + if _, err := tx.Exec(ctx, ` + DELETE FROM email_message_map m + USING unibox_pending_emails p + WHERE p.email_account_id = $1 AND p.payload->'message'->>'folder_path' = ANY($2) + AND m.email_id = p.email_account_id AND m.message_id = p.payload->'message'->>'message_id'`, emailID, folderPaths); err != nil { + return 0, err + } if _, err := tx.Exec(ctx, ` DELETE FROM unibox_pending_emails WHERE email_account_id = $1 AND payload->'message'->>'folder_path' = ANY($2)`, emailID, folderPaths); err != nil { diff --git a/web/src/components/app/emails/SyncStatusCard.tsx b/web/src/components/app/emails/SyncStatusCard.tsx index 1b3f1a7d8..a3bbd0b36 100644 --- a/web/src/components/app/emails/SyncStatusCard.tsx +++ b/web/src/components/app/emails/SyncStatusCard.tsx @@ -4,6 +4,7 @@ import { CheckCircle2Icon, DownloadIcon, HourglassIcon, PlusIcon, RefreshCwIcon import toast from "react-hot-toast"; import { CheckSquare } from "@/components/ui/check-square"; import { TextInput } from "@/components/ui/field"; +import { useConfirm } from "@/hooks/context/confirm"; import type { AppError } from "@/lib/api/client/normalizeError"; import useSync from "@/lib/api/hooks/app/emails/useSync"; import useUpdateSyncSkipFolders from "@/lib/api/hooks/app/emails/useUpdateSyncSkipFolders"; @@ -55,6 +56,7 @@ function skippable(folders: SyncFolder[]): string[] { // synced) can be typed in. function SkipFoldersSection({ mailboxId, listed, skipped }: { mailboxId: string; listed: string[]; skipped: string[] }) { const mutation = useUpdateSyncSkipFolders(mailboxId); + const confirm = useConfirm(); const [draft, setDraft] = useState(""); const isSkipped = (name: string) => skipped.some((s) => s.toLowerCase() === name.toLowerCase()); @@ -67,14 +69,26 @@ function SkipFoldersSection({ mailboxId, listed, skipped }: { mailboxId: string; toast.error(buildError(e as AppError)); } }; - const toggle = (name: string) => - save(isSkipped(name) ? skipped.filter((s) => s.toLowerCase() !== name.toLowerCase()) : [...skipped, name]); + // Skipping removes what was imported and nothing brings it back, so it + // asks first; following a folder again does not. + const skip = (name: string) => + confirm.show( + `Stop syncing "${name}"? Mail already imported from it is removed from Warmbly. It stays in your mailbox, and is not imported again if you turn the folder back on.`, + () => save([...skipped, name]), + ); + const toggle = (name: string) => { + if (isSkipped(name)) { + void save(skipped.filter((s) => s.toLowerCase() !== name.toLowerCase())); + } else { + skip(name); + } + }; const add = () => { const name = draft.trim(); if (!name) return; setDraft(""); if (isSkipped(name)) return; - void save([...skipped, name]); + skip(name); }; return (