fix(worker): guard the IMAP client across reconnects; Gmail-only name fallback

Address review:

- The client field was read by commands while ensureConnected could swap
  it. A lifecycle RWMutex now holds the write lock through dial, auth and
  assignment, and every command holds the read lock for its duration.
  Lock order is mu before lifecycle; sentMailbox resolves under mu before
  AppendToSent takes the read lock.
- The virtual-folder name fallback applied to any server, so a plain IMAP
  account with a real folder called "Important" or "Starred" would have
  been dropped from sync. It now applies only inside Gmail's own
  "[Gmail]/" and "[Google Mail]/" namespace; regression cases added.

internal/app/worker/wmail/folder_test.go

@@ -53,9 +53,15 @@ func TestImapVirtualFolder(t *testing.T) {
 		{models.Mailbox{Name: "[Gmail]/All Mail", Attrs: []string{"\\All", "\\HasNoChildren"}}, true},
 		{models.Mailbox{Name: "[Gmail]/Starred", Attrs: []string{"\\Flagged"}}, true},
 		{models.Mailbox{Name: "[Gmail]/Important", Attrs: []string{"\\Important"}}, true},
-		// A plain LIST (no SPECIAL-USE) carries only \HasNoChildren.
+		// A plain LIST (no SPECIAL-USE) carries only \HasNoChildren; the name
+		// fallback applies inside Gmail's namespace only.
 		{models.Mailbox{Name: "[Gmail]/All Mail", Attrs: []string{"\\HasNoChildren"}}, true},
 		{models.Mailbox{Name: "[Gmail]/Starred"}, true},
+		{models.Mailbox{Name: "[Google Mail]/Important"}, true},
+		// Ordinary IMAP folders that happen to share the names are real.
+		{models.Mailbox{Name: "Important"}, false},
+		{models.Mailbox{Name: "INBOX.Starred"}, false},
+		{models.Mailbox{Name: "All Mail"}, false},
 		{models.Mailbox{Name: "[Gmail]/Sent Mail", Attrs: []string{"\\Sent"}}, false},
 		{models.Mailbox{Name: "[Gmail]/Bin", Attrs: []string{"\\Trash"}}, false},
 		{models.Mailbox{Name: "INBOX"}, false},

internal/app/worker/wmail/sync_imap.go

@@ -421,11 +421,13 @@ func imapVirtualFolder(box *models.Mailbox) bool {
 			return true
 		}
 	}
+	// Name fallback only inside Gmail's own namespace: a plain IMAP server
+	// can legitimately have a user folder called "Important" or "Starred".
 	name := strings.ToLower(box.Name)
-	if i := strings.LastIndexAny(name, "/."); i >= 0 {
-		name = name[i+1:]
+	if !strings.HasPrefix(name, "[gmail]/") && !strings.HasPrefix(name, "[google mail]/") {
+		return false
 	}
-	switch name {
+	switch name[strings.Index(name, "/")+1:] {
 	case "all mail", "starred", "important":
 		return true
 	}

internal/client/smtpimap/imap/append_sent.go

@@ -34,6 +34,8 @@ func (c *Client) AppendToSent(ctx context.Context, raw []byte, sentAt time.Time)
 		return merr
 	}
 
+	// Resolved before the read lock: sentMailbox takes mu, and mu is ordered
+	// before lifecycle.
 	mailbox, err := c.sentMailbox()
 	if err != nil {
 		return err
@@ -42,6 +44,8 @@ func (c *Client) AppendToSent(ctx context.Context, raw []byte, sentAt time.Time)
 	if sentAt.IsZero() {
 		sentAt = time.Now()
 	}
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 	cmd := c.client.Append(mailbox, int64(len(raw)), &imap.AppendOptions{
 		// The sender has, by definition, read what they just sent.
 		Flags: []imap.Flag{imap.FlagSeen},
@@ -71,6 +75,8 @@ func (c *Client) sentMailbox() (string, error) {
 	if c.sentMailboxName != "" {
 		return c.sentMailboxName, nil
 	}
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 
 	// RETURN (SPECIAL-USE) is only legal when the server advertises it; without
 	// the capability the attributes may still arrive on an ordinary LIST.

internal/client/smtpimap/imap/client.go

@@ -63,28 +63,39 @@ type Client struct {
 	// route is used.
 	BindIP *net.TCPAddr
 
-	// reconnectMu serializes ensureConnected between the sync loop (which does
-	// not hold mu) and the send/warmup paths (which do), so a drop seen by both
-	// at once dials one new session, not two.
-	reconnectMu sync.Mutex
+	// lifecycle guards the client field itself. A reconnect holds the write
+	// lock through dial, auth and assignment; every command holds the read
+	// lock for its duration, so a reconnect never swaps the session out from
+	// under a command, and two paths that both see the drop dial once.
+	// Lock order: mu before lifecycle, and never nest a read lock.
+	lifecycle sync.RWMutex
 }
 
 // ensureConnected re-dials after the server has dropped the session. go-imap
 // parks a dead client in the Logout state and fails every later command with
 // net.ErrClosed; nothing re-dialed, so one drop (Gmail closes sessions after a
 // while) left the mailbox a zombie until the worker restarted: no sync, no
-// sent copies. Every entry point that starts a command runs through here.
+// sent copies. Every entry point that starts a command runs through here,
+// before taking its own read lock.
 func (c *Client) ensureConnected() *errx.MailError {
-	c.reconnectMu.Lock()
-	defer c.reconnectMu.Unlock()
+	c.lifecycle.Lock()
+	defer c.lifecycle.Unlock()
 
 	if c.client != nil && c.client.State() != imap.ConnStateLogout {
 		return nil
 	}
-	return c.Connect()
+	return c.connectLocked()
 }
 
 func (c *Client) Connect() *errx.MailError {
+	c.lifecycle.Lock()
+	defer c.lifecycle.Unlock()
+	return c.connectLocked()
+}
+
+// connectLocked dials and authenticates a fresh session. lifecycle must be
+// held for writing.
+func (c *Client) connectLocked() *errx.MailError {
 	var addr, host, security string
 	var port int
 	switch c.AuthType {
@@ -147,6 +158,11 @@ func (c *Client) Connect() *errx.MailError {
 }
 
 func (c *Client) Close() error {
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
+	if c.client == nil {
+		return nil
+	}
 	return c.client.Close()
 }
 
@@ -190,6 +206,8 @@ func (c *Client) Folders() ([]models.Mailbox, *errx.MailError) {
 	if err := c.ensureConnected(); err != nil {
 		return nil, err
 	}
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 
 	// LIST-STATUS: without requesting these, f.Status is nil for every
 	// folder and the sync loop sees an empty account.
@@ -242,6 +260,8 @@ func (c *Client) Folders() ([]models.Mailbox, *errx.MailError) {
 }
 
 func (c *Client) Mailbox(mailbox string, uidvali, opts *imap.SelectOptions) error {
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 	if _, err := c.selectMailbox(mailbox, opts); err != nil {
 		return err
 	}
@@ -252,7 +272,7 @@ func (c *Client) Mailbox(mailbox string, uidvali, opts *imap.SelectOptions) erro
 // selectMailbox is the single SELECT funnel: every path that changes the
 // selected mailbox goes through it so ReleaseMailbox knows whether there is
 // one to release. A failed SELECT leaves the session with no mailbox
-// selected (RFC 3501 6.3.1).
+// selected (RFC 3501 6.3.1). The caller holds the lifecycle read lock.
 func (c *Client) selectMailbox(mailbox string, opts *imap.SelectOptions) (*imap.SelectData, error) {
 	data, err := c.client.Select(mailbox, opts).Wait()
 	c.selected.Store(err == nil)
@@ -265,6 +285,8 @@ func (c *Client) selectMailbox(mailbox string, opts *imap.SelectOptions) (*imap.
 // what arms ChangedSince. The count lets the caller skip the fetch entirely
 // for an empty mailbox, where a 1:* set is a server error.
 func (c *Client) SelectForSync(mailbox string) (uint32, *errx.MailError) {
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 	data, err := c.selectMailbox(mailbox, &imap.SelectOptions{ReadOnly: true, CondStore: true})
 	if err != nil {
 		return 0, c.handleError(err)
@@ -282,6 +304,8 @@ func (c *Client) SelectForSync(mailbox string) (uint32, *errx.MailError) {
 func (c *Client) ReleaseMailbox() {
 	c.mu.Lock()
 	defer c.mu.Unlock()
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 
 	if c.client == nil || !c.selected.Load() || !c.client.Caps().Has(imap.CapUnselect) {
 		return
@@ -317,6 +341,8 @@ func (c *Client) SearchChangedSince(modSeq uint64) ([]imap.UID, *errx.MailError)
 }
 
 func (c *Client) uidSearch(criteria *imap.SearchCriteria) ([]imap.UID, *errx.MailError) {
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 	data, err := c.client.UIDSearch(criteria, nil).Wait()
 	if err != nil {
 		return nil, c.handleError(err)
@@ -335,6 +361,8 @@ func (c *Client) FetchEnvelopes(ctx context.Context, uids []imap.UID) ([]*Fetche
 	for _, uid := range uids {
 		set.AddNum(uid)
 	}
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 	cmd := c.client.Fetch(set, &imap.FetchOptions{
 		UID:      true,
 		Envelope: true,
@@ -417,6 +445,8 @@ func (c *Client) FetchBody(f *Fetched) {
 	if f == nil || f.Email == nil {
 		return
 	}
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 	f.Email.BodyPlain, f.Email.BodyHTML = fetchTextParts(c.client, f.uid, f.body)
 }
 

internal/client/smtpimap/imap/warmup_actions.go

@@ -18,6 +18,8 @@ func (c *Client) MarkAsRead(ctx context.Context, mailboxName string, uid uint32)
 	if merr := c.ensureConnected(); merr != nil {
 		return merr
 	}
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 	if _, err := c.selectMailbox(mailboxName, nil); err != nil {
 		return fmt.Errorf("select %q: %w", mailboxName, err)
 	}
@@ -42,6 +44,8 @@ func (c *Client) MarkImportant(ctx context.Context, mailboxName string, uid uint
 	if merr := c.ensureConnected(); merr != nil {
 		return merr
 	}
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 	if _, err := c.selectMailbox(mailboxName, nil); err != nil {
 		return fmt.Errorf("select %q: %w", mailboxName, err)
 	}
@@ -76,6 +80,8 @@ func (c *Client) MoveToFolder(ctx context.Context, sourceMailbox, dstFolder stri
 	if merr := c.ensureConnected(); merr != nil {
 		return merr
 	}
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 	dst := c.qualifyMailboxLocked(dstFolder)
 	if err := c.ensureMailboxExists(dst); err != nil {
 		return err
@@ -119,6 +125,8 @@ func (c *Client) moveUID(ctx context.Context, src, dst string, uid uint32) error
 	if merr := c.ensureConnected(); merr != nil {
 		return merr
 	}
+	c.lifecycle.RLock()
+	defer c.lifecycle.RUnlock()
 	return c.moveUIDLocked(src, dst, uid)
 }