From 761114ad03dfb8e8aac2d9255c77fd51c66c1e68 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 8 May 2026 03:52:20 -0700 Subject: [PATCH 1/2] fix(config): GetCachedEmailBody no longer mutates cache state Closes #1251 GetCachedEmailBody is a read operation but updated LastAccessedAt and rewrote the cache file on every hit, violating the read-doesn't-mutate boundary called out in #1251. main.go calls GetCachedEmailBody twice per email view (in UpdatePreviewMsg and ViewEmailMsg) and both call sites already chase the cached read with SaveEmailBody, which is the function responsible for stamping LastAccessedAt and persisting. Removing the duplicate work here also removes one disk write per email view. Drops the two lines flagged in the issue (config/cache.go:505-506) and adds a one-line note above the function pointing readers at SaveEmailBody as the access-time owner. --- config/cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/cache.go b/config/cache.go index 0774516..04979e2 100644 --- a/config/cache.go +++ b/config/cache.go @@ -495,6 +495,8 @@ func saveEmailBodyCache(cache *EmailBodyCache) error { } // GetCachedEmailBody returns the cached body for a specific email, or nil if not cached. +// LastAccessedAt is updated by SaveEmailBody, not here -- a read should not +// mutate cache state. func GetCachedEmailBody(folderName string, uid uint32, accountID string) *CachedEmailBody { cache, err := LoadEmailBodyCache(folderName) if err != nil { @@ -502,8 +504,6 @@ func GetCachedEmailBody(folderName string, uid uint32, accountID string) *Cached } for i, b := range cache.Bodies { if b.UID == uid && b.AccountID == accountID { - cache.Bodies[i].LastAccessedAt = time.Now() - _ = saveEmailBodyCache(cache) return &cache.Bodies[i] } } From 70be01db6b5eed226367ff528bd6730d07543fd6 Mon Sep 17 00:00:00 2001 From: Matt Van Horn Date: Sun, 10 May 2026 03:45:11 -0700 Subject: [PATCH 2/2] fix(tui): surface per-account folder fetch errors instead of silently dropping fetchFoldersCmd spawns one goroutine per account, each calling fetcher.FetchFolders. The previous implementation discarded any error returned by FetchFolders -- on broken IMAP login, expired OAuth, or unreachable server, the affected account silently dropped out of the merged folder list and the user got no signal. The reporter (issue #1125) flagged this as a UX bug: the user sees their folder list miss entries with no explanation. Collect per-account errors into a sibling map and pass them through FoldersFetchedMsg so the TUI can decide what to do with them. The consumer at main.go:510 now surfaces non-empty errors as a transient overlay (4s, then auto-restore) using the same PluginNotifyMsg pattern the rest of the TUI uses for transient status: save the previous model, swap to a Status overlay, fire RestoreViewMsg via tea.Tick. Account display name lookup prefers Email -> Name -> ID so the error string shows something meaningful even on partially configured accounts. Errors are sorted alphabetically so the overlay is deterministic across restarts. The disjoint-by-construction property (an account either populates FoldersByAccount or Errors, never both) is preserved by the existing return-after-error in the goroutine. Closes #1125. Co-Authored-By: Claude Opus 4.7 (1M context) --- main.go | 43 +++++++++++++++++++++++++++++++++++++++++++ tui/messages.go | 7 +++++++ 2 files changed, 50 insertions(+) diff --git a/main.go b/main.go index 6e683d1..e9c9c23 100644 --- a/main.go +++ b/main.go @@ -524,6 +524,44 @@ func (m *mainModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } go config.SaveAccountFolders(accID, names) } + // Per-account fetch errors (e.g. broken IMAP login, unreachable + // server) are non-fatal: other accounts' folders are still shown. + // Surface them as a transient overlay so the user knows why an + // account's folders are missing instead of silently dropping them. + // Reuses the PluginNotifyMsg pattern (save current view, show + // status with a tea.Tick that fires RestoreViewMsg). + if len(msg.Errors) > 0 { + lookup := map[string]string{} + if m.config != nil { + for _, acc := range m.config.Accounts { + name := acc.Email + if name == "" { + name = acc.Name + } + if name == "" { + name = acc.ID + } + lookup[acc.ID] = name + } + } + parts := make([]string, 0, len(msg.Errors)) + for accID, err := range msg.Errors { + name := lookup[accID] + if name == "" { + name = accID + } + parts = append(parts, fmt.Sprintf("%s: %v", name, err)) + } + sort.Strings(parts) + m.previousModel = m.current + m.current = tui.NewStatus(fmt.Sprintf( + "Folder fetch failed for %d account(s): %s", + len(parts), strings.Join(parts, "; "), + )) + return m, tea.Tick(4*time.Second, func(t time.Time) tea.Msg { + return tui.RestoreViewMsg{} + }) + } return m, nil case tui.SwitchFolderMsg: @@ -2675,6 +2713,7 @@ func fetchFoldersCmd(cfg *config.Config) tea.Cmd { return nil } foldersByAccount := make(map[string][]fetcher.Folder) + errsByAccount := make(map[string]error) seen := make(map[string]fetcher.Folder) var mu sync.Mutex var wg sync.WaitGroup @@ -2685,6 +2724,9 @@ func fetchFoldersCmd(cfg *config.Config) tea.Cmd { defer wg.Done() folders, err := fetcher.FetchFolders(&acc) if err != nil { + mu.Lock() + errsByAccount[acc.ID] = err + mu.Unlock() return } mu.Lock() @@ -2707,6 +2749,7 @@ func fetchFoldersCmd(cfg *config.Config) tea.Cmd { return tui.FoldersFetchedMsg{ FoldersByAccount: foldersByAccount, MergedFolders: merged, + Errors: errsByAccount, } } } diff --git a/tui/messages.go b/tui/messages.go index 8a605eb..207ecc5 100644 --- a/tui/messages.go +++ b/tui/messages.go @@ -416,9 +416,16 @@ type RequestRefreshMsg struct { // --- Folder Messages --- // FoldersFetchedMsg signals that IMAP folders have been fetched for all accounts. +// +// Errors holds per-account fetch failures (e.g. broken IMAP login, network +// unreachable). Accounts that succeeded appear in FoldersByAccount; accounts +// that failed appear in Errors. The two are disjoint by construction. This +// lets the TUI surface a non-fatal warning instead of silently dropping the +// affected account's folder list. type FoldersFetchedMsg struct { FoldersByAccount map[string][]fetcher.Folder // accountID -> folders MergedFolders []fetcher.Folder // unique folders across all accounts + Errors map[string]error // accountID -> fetch error, if any } // SwitchFolderMsg signals switching to a different IMAP folder.