fix: Properly handle store flags (when replacing)

internal/state/actions.go

@@ -435,49 +435,33 @@ func (state *State) actionSetMessageFlags(ctx context.Context, tx *ent.Tx, messa
 		return sm.ID.InternalID
 	})
 
-	// If setting messages as seen, only set those messages that aren't currently seen.
-	var messagesToApply []imap.MessageID
+	// If setting messages as seen, only set those messages that aren't currently seen, and vice versa.
+	setSeen := map[bool][]imap.MessageID{true: {}, false: {}}
 
 	for _, msg := range messages {
-		if msg.flags.ContainsUnchecked(imap.FlagSeenLowerCase) {
-			messagesToApply = append(messagesToApply, msg.ID.RemoteID)
+		if seen := setFlags.ContainsUnchecked(imap.FlagSeenLowerCase); seen != msg.flags.ContainsUnchecked(imap.FlagSeenLowerCase) {
+			setSeen[seen] = append(setSeen[seen], msg.ID.RemoteID)
 		}
 	}
 
-	if len(messagesToApply) != 0 {
-		if err := state.user.GetRemote().SetMessagesSeen(ctx, messagesToApply, false); err != nil {
+	for seen, messageIDs := range setSeen {
+		if err := state.user.GetRemote().SetMessagesSeen(ctx, messageIDs, seen); err != nil {
 			return err
 		}
 	}
 
-	// If setting messages as flagged, only set those messages that aren't currently flagged.
-	if setFlags.ContainsUnchecked(imap.FlagFlaggedLowerCase) {
-		var messagesToApply []imap.MessageID
-
-		for _, msg := range messages {
-			if !msg.flags.ContainsUnchecked(imap.FlagFlaggedLowerCase) {
-				messagesToApply = append(messagesToApply, msg.ID.RemoteID)
-			}
-		}
-
-		if len(messagesToApply) != 0 {
-			if err := state.user.GetRemote().SetMessagesFlagged(ctx, messagesToApply, true); err != nil {
-				return err
-			}
-		}
-	} else {
-		var messagesToApply []imap.MessageID
+	// If setting messages as flagged, only set those messages that aren't currently flagged, and vice versa.
+	setFlagged := map[bool][]imap.MessageID{true: {}, false: {}}
 
-		for _, msg := range messages {
-			if msg.flags.ContainsUnchecked(imap.FlagFlagged) {
-				messagesToApply = append(messagesToApply, msg.ID.RemoteID)
-			}
+	for _, msg := range messages {
+		if flagged := setFlags.ContainsUnchecked(imap.FlagFlaggedLowerCase); flagged != msg.flags.ContainsUnchecked(imap.FlagFlaggedLowerCase) {
+			setFlagged[flagged] = append(setFlagged[flagged], msg.ID.RemoteID)
 		}
+	}
 
-		if len(messagesToApply) != 0 {
-			if err := state.user.GetRemote().SetMessagesFlagged(ctx, messagesToApply, false); err != nil {
-				return err
-			}
+	for flagged, messageIDs := range setFlagged {
+		if err := state.user.GetRemote().SetMessagesFlagged(ctx, messageIDs, flagged); err != nil {
+			return err
 		}
 	}
 

internal/state/updates.go

@@ -233,7 +233,7 @@ func (u *messageFlagsSetStateUpdate) Apply(ctx context.Context, tx *ent.Tx, stat
 }
 
 func (u *messageFlagsSetStateUpdate) String() string {
-	return fmt.Sprintf("MessagFlagsSetStateUpdate: mbox = %v messages = %v flags=%v",
+	return fmt.Sprintf("MessageFlagsSetStateUpdate: mbox = %v messages = %v flags=%v",
 		u.mboxID.InternalID.ShortID(),
 		xslices.Map(u.messageIDs, func(id imap.InternalMessageID) string {
 			return id.ShortID()

tests/store_test.go

@@ -1,6 +1,7 @@
 package tests
 
 import (
+	"fmt"
 	"testing"
 
 	goimap "github.com/emersion/go-imap"
@@ -116,6 +117,41 @@ func TestStoreSilent(t *testing.T) {
 	})
 }
 
+func TestStoreReadUnread(t *testing.T) {
+	runManyToOneTestWithAuth(t, defaultServerOptions(t), []int{1, 2}, func(c map[int]*testConnection, _ *testSession) {
+		c[1].C(`tag select inbox`).OK(`tag`)
+		c[2].C(`tag select inbox`).OK(`tag`)
+
+		for i := 1; i <= 10; i++ {
+			c[1].doAppend(`INBOX`, fmt.Sprintf(`To: %[email protected]`, i)).expect("OK")
+
+			c[1].C(`tag noop`).OK(`tag`)
+			c[2].C(`tag noop`).OK(`tag`)
+		}
+
+		for i := 1; i <= 10; i++ {
+			// Begin IDLEing on session 2.
+			c[2].C(`tag idle`).Continue()
+
+			// Mark the message as read with session 1.
+			c[1].Cf(`tag UID STORE %v +FLAGS.SILENT (\Seen)`, i).OK(`tag`)
+
+			// Mark the message as read with session 1.
+			c[1].Cf(`tag UID STORE %v FLAGS.SILENT (\Seen)`, i).OK(`tag`)
+
+			// Wait for the untagged FETCH response on session 2.
+			c[2].S(fmt.Sprintf(`* %v FETCH (FLAGS (\Seen))`, i))
+
+			// End IDLEing on session 2.
+			c[2].C(`DONE`).OK(`tag`)
+
+			// Both sessions should see the message as read.
+			c[1].Cf(`tag UID FETCH %v (FLAGS)`, i).Sxe(`\Seen`).OK(`tag`)
+			c[2].Cf(`tag UID FETCH %v (FLAGS)`, i).Sxe(`\Seen`).OK(`tag`)
+		}
+	})
+}
+
 func TestUIDStore(t *testing.T) {
 	runOneToOneTestWithAuth(t, defaultServerOptions(t), func(c *testConnection, _ *testSession) {
 		c.C("b001 CREATE saved-messages")