app/src/main/kotlin/org/libremail/mail/ImapClient.kt

@@ -5,6 +5,7 @@ import jakarta.mail.FetchProfile
 import jakarta.mail.Flags
 import jakarta.mail.Folder
 import jakarta.mail.Message
+import jakarta.mail.MessagingException
 import jakarta.mail.Multipart
 import jakarta.mail.Part
 import jakarta.mail.Session
@@ -103,6 +104,16 @@ data class ReplyContext(
 class ImapClient internal constructor(
     private val reuseConnections: Boolean,
     private val reuseIdleTimeoutMillis: Long = DEFAULT_REUSE_IDLE_TIMEOUT_MS,
+    /**
+     * Reports whether the folder's already-open IMAP connection advertises the UIDPLUS extension (RFC
+     * 4315), which gates the targeted `UID EXPUNGE` vs. the plain-EXPUNGE fallback in [expungeTargeted]
+     * (issue #319). Reads the capability from the folder's own protocol so it never opens a second
+     * connection + LOGIN mid-operation — preserving the one-connection-per-batch invariant of issues
+     * #125/#295. Defaults to the real probe ([probeUidPlusCapability]); the internal constructor lets a
+     * test inject a fixed value, because GreenMail always advertises UIDPLUS and so cannot exercise the
+     * no-UIDPLUS fallback path on its own.
+     */
+    private val supportsUidPlus: (IMAPFolder) -> Boolean = ::probeUidPlusCapability,
 ) {
 
     /**
@@ -432,17 +443,61 @@ class ImapClient internal constructor(
     }
 
     /**
-     * Permanently removes exactly [messages] — which the caller has already flagged `\Deleted` — with a
-     * **targeted** UID EXPUNGE (RFC 4315, [IMAPFolder.expunge]). Deliberately never the untargeted
-     * [Folder.expunge], which expunges *every* `\Deleted`-flagged message in [mailbox] — including ones
-     * a second client, Gmail, or a partial earlier move left flagged — the data-loss bug in issue #295.
-     * Every folder this client opens is an [IMAPFolder], so the cast holds in production and under
-     * GreenMail. The server must advertise UIDPLUS (Gmail, Outlook, and GreenMail do); on one that does
-     * not, Angus raises "UID EXPUNGE not supported" here rather than silently falling back to the
-     * unrelated-mail-destroying untargeted expunge — a loud failure is the correct, safe outcome.
+     * Permanently removes exactly [messages] — which the caller has already flagged `\Deleted` — using a
+     * **targeted** UID EXPUNGE (RFC 4315, [IMAPFolder.expunge]) whenever the server advertises UIDPLUS
+     * (Gmail, Outlook, and GreenMail do). That never touches other `\Deleted`-flagged mail in [mailbox]
+     * — ones a second client, Gmail, or a partial earlier move left flagged — the data-loss bug in
+     * issue #295.
+     *
+     * On a server **without** UIDPLUS the targeted UID EXPUNGE throws `UID EXPUNGE not supported`, which
+     * used to break delete/move entirely (issue #319). There we fall back to a plain, untargeted
+     * [Folder.expunge] — but **only** when it is provably safe, i.e. the messages we just flagged are the
+     * *only* `\Deleted` ones in [mailbox] ([countForeignDeletedMessages] is 0). A plain EXPUNGE removes
+     * **every** `\Deleted` message, so when unrelated `\Deleted` mail is present we refuse and fail loud
+     * rather than destroy mail the user never selected: there is no UIDPLUS-free way to expunge one
+     * specific UID while sparing the others (that capability is exactly what UIDPLUS provides), so
+     * preserving the issue-#295 invariant — never touch unrelated `\Deleted` mail — wins over completing
+     * the delete. Every folder this client opens is an [IMAPFolder], so the cast holds in production and
+     * under GreenMail.
      */
     private fun expungeTargeted(mailbox: Folder, messages: Array<Message>) {
-        (mailbox as IMAPFolder).expunge(messages)
+        val imapFolder = mailbox as IMAPFolder
+        if (supportsUidPlus(imapFolder)) {
+            imapFolder.expunge(messages)
+            return
+        }
+        val foreignDeleted = countForeignDeletedMessages(imapFolder, messages)
+        if (foreignDeleted == 0) {
+            AppLog.i(EXPUNGE_TAG, "server lacks UIDPLUS; no other \\Deleted mail present, using safe plain EXPUNGE")
+            imapFolder.expunge()
+        } else {
+            AppLog.w(
+                EXPUNGE_TAG,
+                "server lacks UIDPLUS and $foreignDeleted other \\Deleted message(s) present; " +
+                    "refusing plain EXPUNGE so unrelated mail is not destroyed",
+            )
+            throw MessagingException(
+                "Cannot honor a targeted expunge: the server does not support UIDPLUS and other deleted " +
+                    "messages are present, so a plain EXPUNGE would remove mail that was not selected",
+            )
+        }
+    }
+
+    /**
+     * Counts messages in [mailbox] flagged `\Deleted` that are **not** among [targets] (the ones the
+     * caller just flagged). Used only on the no-UIDPLUS fallback in [expungeTargeted] to decide whether a
+     * plain EXPUNGE is safe — it is safe exactly when this is 0. Fetches FLAGS + UID for the whole folder,
+     * acceptable on this rare legacy-server path.
+     */
+    private fun countForeignDeletedMessages(mailbox: IMAPFolder, targets: Array<Message>): Int {
+        val targetUids = targets.mapTo(HashSet()) { mailbox.getUID(it) }
+        val all = mailbox.messages
+        val profile = FetchProfile().apply {
+            add(FetchProfile.Item.FLAGS)
+            add(UIDFolder.FetchProfileItem.UID)
+        }
+        mailbox.fetch(all, profile)
+        return all.count { it.isSet(Flags.Flag.DELETED) && mailbox.getUID(it) !in targetUids }
     }
 
     /**
@@ -719,6 +774,7 @@ class ImapClient internal constructor(
         const val TIMEOUT_MS = "15000"
         const val TAG = "LibreMailIdle"
         const val PERF_TAG = "ImapPerf"
+        const val EXPUNGE_TAG = "ImapExpunge"
         const val NANOS_PER_MS = 1_000_000L
 
         // A reused connection unused for this long is idle-evicted (issue #357 Part 2): long enough to
@@ -728,6 +784,21 @@ class ImapClient internal constructor(
     }
 }
 
+/** The IMAP UIDPLUS extension name (RFC 4315); its presence is what makes a targeted `UID EXPUNGE` legal. */
+private const val CAP_UIDPLUS = "UIDPLUS"
+
+/**
+ * Whether [folder]'s already-open connection advertises the IMAP UIDPLUS extension (RFC 4315) — the
+ * capability a targeted `UID EXPUNGE` requires. Reads it from the folder's own protocol via
+ * [IMAPFolder.doCommand] (the capabilities parsed at LOGIN; no extra round trip), reusing the open
+ * connection rather than borrowing a fresh store protocol — which would open a second connection + LOGIN
+ * mid-batch and defeat the reuse invariant of issue #125. Any read failure degrades to `false`, so the
+ * caller takes the safe plain-EXPUNGE fallback. The production default of [ImapClient.supportsUidPlus].
+ */
+private fun probeUidPlusCapability(folder: IMAPFolder): Boolean = runCatching {
+    folder.doCommand { protocol -> protocol.hasCapability(CAP_UIDPLUS) } as? Boolean
+}.getOrNull() ?: false
+
 /**
  * True when [part] is a user-facing downloadable attachment: its `Content-Disposition` is
  * `attachment`, OR it has a filename but no `Content-ID` header. A part with a filename AND a

app/src/test/kotlin/org/libremail/mail/ImapClientTest.kt

@@ -364,6 +364,77 @@ class ImapClientTest {
         assertTrue(archive.contains("Move me"), "archive=$archive")
     }
 
+    // --- issue #319: graceful fallback when the server lacks the UIDPLUS extension ---
+    //
+    // GreenMail always advertises UIDPLUS, so the "with UIDPLUS" case (a targeted UID EXPUNGE that spares
+    // unrelated \Deleted mail) is exercised by the default-client delete/move tests above. To drive the
+    // "without UIDPLUS" branch against the same real server, these tests inject a capability probe that
+    // reports no UIDPLUS while every EXPUNGE below still runs for real against GreenMail.
+
+    private val noUidPlusClient = ImapClient(reuseConnections = false, supportsUidPlus = { false })
+
+    @Test
+    fun `deleteMessages falls back to a safe plain expunge when the server lacks UIDPLUS`() = runTest {
+        val buffer = RingLogBuffer()
+        AppLog.install(buffer)
+        GreenMailUtil.sendTextEmailTest("alice@example.org", "bob@example.org", "Target", "delete this")
+        GreenMailUtil.sendTextEmailTest("alice@example.org", "bob@example.org", "Keep", "no flag at all")
+        greenMail.waitForIncomingEmail(2)
+        val bySubject = client.fetchRecent(params(), "INBOX", limit = 50).associateBy { it.subject }
+
+        // No other \Deleted mail is present, so a plain EXPUNGE removes only the target — and must not throw.
+        noUidPlusClient.deleteMessages(params(), "INBOX", listOf(bySubject.getValue("Target").uid))
+
+        val remaining = client.fetchRecent(params(), "INBOX", limit = 50).map { it.subject }
+        assertFalse(remaining.contains("Target"), "the target must be expunged via the fallback, remaining=$remaining")
+        assertTrue(remaining.contains("Keep"), "an unflagged message must survive, remaining=$remaining")
+        assertTrue(
+            buffer.snapshot().any { it.message.contains("using safe plain EXPUNGE") },
+            "the no-UIDPLUS fallback decision must be logged, messages=${buffer.snapshot().map { it.message }}",
+        )
+    }
+
+    @Test
+    fun `deleteMessages without UIDPLUS refuses to expunge when other Deleted mail is present`() = runTest {
+        val buffer = RingLogBuffer()
+        AppLog.install(buffer)
+        GreenMailUtil.sendTextEmailTest("alice@example.org", "bob@example.org", "Target", "delete this")
+        GreenMailUtil.sendTextEmailTest("alice@example.org", "bob@example.org", "Bystander", "flagged elsewhere")
+        greenMail.waitForIncomingEmail(2)
+        val bySubject = client.fetchRecent(params(), "INBOX", limit = 50).associateBy { it.subject }
+        // A second client left "Bystander" flagged \Deleted but un-expunged; a plain EXPUNGE would drop it.
+        client.setFlag(params(), "INBOX", bySubject.getValue("Bystander").uid, Flags.Flag.DELETED, value = true)
+
+        // Without UIDPLUS there is no way to expunge only "Target" while sparing "Bystander": refuse loudly
+        // rather than destroy mail the user never selected (preserves the issue #295 invariant).
+        assertFailsWith<Exception> {
+            noUidPlusClient.deleteMessages(params(), "INBOX", listOf(bySubject.getValue("Target").uid))
+        }
+
+        val remaining = client.fetchRecent(params(), "INBOX", limit = 50).map { it.subject }
+        assertTrue(remaining.contains("Target"), "the refused target must NOT be expunged, remaining=$remaining")
+        assertTrue(remaining.contains("Bystander"), "unrelated \\Deleted mail must survive, remaining=$remaining")
+        assertTrue(
+            buffer.snapshot().any { it.message.contains("refusing plain EXPUNGE") },
+            "the refusal must be logged, messages=${buffer.snapshot().map { it.message }}",
+        )
+    }
+
+    @Test
+    fun `moveMessages falls back to a safe plain expunge when the server lacks UIDPLUS`() = runTest {
+        GreenMailUtil.sendTextEmailTest("alice@example.org", "bob@example.org", "Move me", "relocate this")
+        greenMail.waitForIncomingEmail(1)
+        appendMessage("Archive", "carol@example.org", "Seed", "Creates the Archive folder")
+        val uid = client.fetchRecent(params(), "INBOX", limit = 50).first { it.subject == "Move me" }.uid
+
+        noUidPlusClient.moveMessages(params(), "INBOX", listOf(uid), "Archive")
+
+        val inbox = client.fetchRecent(params(), "INBOX", limit = 50).map { it.subject }
+        val archive = client.fetchRecent(params(), "Archive", limit = 50).map { it.subject }
+        assertFalse(inbox.contains("Move me"), "the moved message must leave the source via the fallback, inbox=$inbox")
+        assertTrue(archive.contains("Move me"), "archive=$archive")
+    }
+
     @Test
     fun `fetchRecent returns empty for an empty folder`() = runTest {
         createFolder("Empty")