[verified] fix(imap): use native MOVE (#30)

Author: bscott

CHANGELOG.md

@@ -5,6 +5,15 @@ All notable changes to this project are documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Fixed
+- `mail move`, `mail archive`, and batch move/archive operations now use the
+  server's atomic IMAP `MOVE` command when available. This removes a data-loss
+  path where the previous unqualified `EXPUNGE` could permanently delete
+  unrelated messages already marked `\\Deleted`; the library fallback uses
+  targeted `UID EXPUNGE` on UIDPLUS servers (#28, #29).
+
 ## [0.2.6] - 2026-08-09
 
 Validated against Proton Bridge 3.25.0 before release: the IMAP no-match detection,

internal/imap/client.go

@@ -590,21 +590,44 @@ func (c *Client) CopyMessages(mailbox string, ids []string, destMailbox string)
 	return nil
 }
 
-// copyMatchedNothing reports whether a COPY provably affected no messages.
+// numSetsMatchedNothing reports whether UIDPLUS response data proves an
+// operation matched no messages. MOVE returns imap.NumSet values while COPY
+// returns UIDSet values, so the shared check accepts either representation.
 //
-// The evidence is the COPYUID response code, which is only emitted by servers
-// advertising UIDPLUS (folded into IMAP4rev2). Without that capability an empty
-// SourceUIDs/DestUIDs pair means "the server never told us", not "nothing was
-// copied" — treating it as the latter would fail every successful copy. So we
-// only draw the conclusion when the capability guarantees the data is present.
+// Without UIDPLUS (or IMAP4rev2, which includes it), empty source and
+// destination sets mean "the server never told us," not "nothing matched."
+func (c *Client) numSetsMatchedNothing(source, dest imap.NumSet) bool {
+	if !c.client.Caps().Has(imap.CapUIDPlus) {
+		return false
+	}
+	return numSetEmpty(source) && numSetEmpty(dest)
+}
+
+func numSetEmpty(set imap.NumSet) bool {
+	switch set := set.(type) {
+	case nil:
+		return true
+	case imap.SeqSet:
+		return len(set) == 0
+	case imap.UIDSet:
+		return len(set) == 0
+	default:
+		return false
+	}
+}
+
 func (c *Client) copyMatchedNothing(data *imap.CopyData) bool {
 	if data == nil {
 		return false
 	}
-	if !c.client.Caps().Has(imap.CapUIDPlus) {
+	return c.numSetsMatchedNothing(data.SourceUIDs, data.DestUIDs)
+}
+
+func (c *Client) moveMatchedNothing(data *imapclient.MoveData) bool {
+	if data == nil {
 		return false
 	}
-	return len(data.SourceUIDs) == 0 && len(data.DestUIDs) == 0
+	return c.numSetsMatchedNothing(data.SourceUIDs, data.DestUIDs)
 }
 
 func (c *Client) MoveMessages(mailbox string, ids []string, destMailbox string) error {
@@ -618,40 +641,14 @@ func (c *Client) MoveMessages(mailbox string, ids []string, destMailbox string)
 		return err
 	}
 
-	// Copy to destination. As with CopyMessages, a COPY that matches nothing is
-	// not an error on its own, so verify the COPYUID data before proceeding to
-	// delete from the source — otherwise a move of non-existent UIDs would
-	// silently expunge nothing yet report success.
-	copyCmd := c.client.Copy(numSet, destMailbox)
-	copyData, err := copyCmd.Wait()
+	moveData, err := c.client.Move(numSet, destMailbox).Wait()
 	if err != nil {
-		return fmt.Errorf("failed to copy messages to %s: %w", destMailbox, err)
+		return fmt.Errorf("failed to move messages to %s: %w", destMailbox, err)
 	}
-	if c.copyMatchedNothing(copyData) {
+	if c.moveMatchedNothing(moveData) {
 		return fmt.Errorf("no messages matched the given ID(s) in %s", mailbox)
 	}
 
-	// Delete from source. Count the streamed FETCH responses so a STORE that
-	// modifies nothing is reported rather than silently expunged as success.
-	storeCmd := c.client.Store(numSet, &imap.StoreFlags{
-		Op:    imap.StoreFlagsAdd,
-		Flags: []imap.Flag{imap.FlagDeleted},
-	}, nil)
-	affected := 0
-	for storeCmd.Next() != nil {
-		affected++
-	}
-	if err := storeCmd.Close(); err != nil {
-		return fmt.Errorf("failed to delete from source: %w", err)
-	}
-	if affected == 0 {
-		return fmt.Errorf("no messages matched the given ID(s) in %s", mailbox)
-	}
-
-	if err := c.client.Expunge().Close(); err != nil {
-		return fmt.Errorf("failed to expunge: %w", err)
-	}
-
 	return nil
 }
 

internal/imap/client_affected_test.go

@@ -36,11 +36,16 @@ type imapfixture struct {
 	raw    *imapclient.Client
 }
 
-// newIMAPFixture starts a fresh in-memory server with an INBOX and an Archive
-// mailbox, appends one message to INBOX, and returns a Client bound to it along
-// with the UID of the seeded message.
+// newIMAPFixture starts a fresh IMAP4rev2 in-memory server with an INBOX and
+// Archive mailbox, appends one message to INBOX, and returns a Client bound to
+// it along with the UID of the seeded message.
 func newIMAPFixture(t *testing.T) (*imapfixture, imap.UID) {
 	t.Helper()
+	return newIMAPFixtureWithCaps(t, imap.CapSet{imap.CapIMAP4rev2: {}})
+}
+
+func newIMAPFixtureWithCaps(t *testing.T, caps imap.CapSet) (*imapfixture, imap.UID) {
+	t.Helper()
 
 	memServer := imapmemserver.New()
 	user := imapmemserver.NewUser(testUser, testPass)
@@ -50,8 +55,7 @@ func newIMAPFixture(t *testing.T) (*imapfixture, imap.UID) {
 		NewSession: func(*imapserver.Conn) (imapserver.Session, *imapserver.GreetingData, error) {
 			return memServer.NewSession(), nil, nil
 		},
-		// IMAP4rev2 folds in UIDPLUS/MOVE/ESEARCH, giving us COPYUID data.
-		Caps:         imap.CapSet{imap.CapIMAP4rev2: {}},
+		Caps:         caps,
 		InsecureAuth: true,
 	})
 
@@ -223,6 +227,52 @@ func TestMoveMessagesActuallyMoves(t *testing.T) {
 	}
 }
 
+func TestMoveMessagesDoesNotExpungeUnrelatedDeletedMessage(t *testing.T) {
+	fx, uidA := newIMAPFixture(t)
+	uidB := appendMessage(t, fx.raw, "INBOX", "second@example.com")
+
+	if err := fx.client.DeleteMessages("INBOX", []string{presentSelector(uidA)}, false); err != nil {
+		t.Fatalf("soft delete unrelated message: %v", err)
+	}
+	if err := fx.client.MoveMessages("INBOX", []string{presentSelector(uidB)}, "Archive"); err != nil {
+		t.Fatalf("move other message: %v", err)
+	}
+
+	assertMessageStillInInbox(t, fx.client, uidA, uidB)
+}
+
+func TestMoveMessagesFallbackDoesNotExpungeUnrelatedDeletedMessage(t *testing.T) {
+	caps := imap.CapSet{imap.CapIMAP4rev1: {}, imap.CapUIDPlus: {}}
+	fx, uidA := newIMAPFixtureWithCaps(t, caps)
+	uidB := appendMessage(t, fx.raw, "INBOX", "second@example.com")
+
+	if fx.client.client.Caps().Has(imap.CapMove) {
+		t.Fatal("fixture unexpectedly advertises MOVE; fallback path not exercised")
+	}
+	if err := fx.client.DeleteMessages("INBOX", []string{presentSelector(uidA)}, false); err != nil {
+		t.Fatalf("soft delete unrelated message: %v", err)
+	}
+	if err := fx.client.MoveMessages("INBOX", []string{presentSelector(uidB)}, "Archive"); err != nil {
+		t.Fatalf("fallback move other message: %v", err)
+	}
+
+	assertMessageStillInInbox(t, fx.client, uidA, uidB)
+}
+
+func assertMessageStillInInbox(t *testing.T, client *Client, wantUID, movedUID imap.UID) {
+	t.Helper()
+	inbox, err := client.ListMessages(ListOptions{Mailbox: "INBOX", Limit: 50})
+	if err != nil {
+		t.Fatalf("ListMessages INBOX: %v", err)
+	}
+	for _, message := range inbox {
+		if imap.UID(message.UID) == wantUID {
+			return
+		}
+	}
+	t.Errorf("moving UID %d expunged unrelated deleted UID %d", movedUID, wantUID)
+}
+
 // --- Security -----------------------------------------------------------
 
 // TestMissingUIDDoesNotMutateMailbox ensures a failed (no-match) delete leaves