Fix: Quote mailbox names and add IMAP flag detection for Sent folder (#93)

KonstiDoll · KonstiDoll · web-flow · commit 4f8b616f388f · 2026-01-07T22:19:44.000+08:00
* Fix: Quote mailbox names and add IMAP flag detection for Sent folder

Fixes emails not being saved to Sent folder for providers like IONOS
that use folder names with spaces (e.g., "Gesendete Objekte").

Changes:
- Use _quote_mailbox() in IMAP APPEND command (RFC 3501 compliance)
- Add _find_sent_folder_by_flag() to detect Sent folder via IMAP \Sent flag
- Add error handling around append_to_sent()

Tested with:
- IONOS: Now works correctly
- Gmail: Backward compatible, still works

* Add comprehensive tests for IMAP flag detection and error handling

- Test _find_sent_folder_by_flag() with various folder names (IONOS, Gmail, etc.)
- Test integration with append_to_sent()
- Test error handling when append_to_sent fails
- All 9 new tests passing, total 27/27 tests in test_save_to_sent.py

---------

Co-authored-by: KonstiDoll &lt;konstantin.doll@urban-codesign.com&gt;
diff --git a/mcp_email_server/emails/classic.py b/mcp_email_server/emails/classic.py
@@ -638,6 +638,37 @@ async def send_email(
         # Return the message for potential saving to Sent folder
         return msg
 
+    async def _find_sent_folder_by_flag(self, imap) -> str | None:
+        """Find the Sent folder by searching for the \\Sent IMAP flag.
+
+        Args:
+            imap: Connected IMAP client
+
+        Returns:
+            The folder name with the \\Sent flag, or None if not found
+        """
+        try:
+            # List all folders - aioimaplib requires reference_name and mailbox_pattern
+            _, folders = await imap.list('""', "*")
+
+            # Search for folder with \Sent flag
+            for folder in folders:
+                folder_str = folder.decode("utf-8") if isinstance(folder, bytes) else str(folder)
+                # IMAP LIST response format: (flags) "delimiter" "name"
+                # Example: (\Sent \HasNoChildren) "/" "Gesendete Objekte"
+                if r"\Sent" in folder_str or "\\Sent" in folder_str:
+                    # Extract folder name from the response
+                    # Split by quotes and get the last quoted part
+                    parts = folder_str.split('"')
+                    if len(parts) >= 3:
+                        folder_name = parts[-2]  # The folder name is the second-to-last quoted part
+                        logger.info(f"Found Sent folder by \\Sent flag: '{folder_name}'")
+                        return folder_name
+        except Exception as e:
+            logger.debug(f"Error finding Sent folder by flag: {e}")
+
+        return None
+
     async def append_to_sent(
         self,
         msg: MIMEText | MIMEMultipart,
@@ -676,6 +707,12 @@ async def append_to_sent(
             await imap.login(incoming_server.user_name, incoming_server.password)
             await _send_imap_id(imap)
 
+            # Try to find Sent folder by IMAP \Sent flag first
+            flag_folder = await self._find_sent_folder_by_flag(imap)
+            if flag_folder and flag_folder not in sent_folder_candidates:
+                # Add it at the beginning (high priority)
+                sent_folder_candidates.insert(0, flag_folder)
+
             # Try to find and use the Sent folder
             for folder in sent_folder_candidates:
                 try:
@@ -693,7 +730,7 @@ async def append_to_sent(
                         # aioimaplib.append signature: (message_bytes, mailbox, flags, date)
                         append_result = await imap.append(
                             msg_bytes,
-                            mailbox=folder,
+                            mailbox=_quote_mailbox(folder),
                             flags=r"(\Seen)",
                         )
                         logger.debug(f"Append result: {append_result}")
@@ -845,11 +882,14 @@ async def send_email(
 
         # Save to Sent folder if enabled
         if self.save_to_sent and msg:
-            await self.outgoing_client.append_to_sent(
-                msg,
-                self.email_settings.incoming,
-                self.sent_folder_name,
-            )
+            try:
+                await self.outgoing_client.append_to_sent(
+                    msg,
+                    self.email_settings.incoming,
+                    self.sent_folder_name,
+                )
+            except Exception as e:
+                logger.error(f"Failed to save email to Sent folder: {e}", exc_info=True)
 
     async def delete_emails(self, email_ids: list[str], mailbox: str = "INBOX") -> tuple[list[str], list[str]]:
         """Delete emails by their UIDs. Returns (deleted_ids, failed_ids)."""
diff --git a/tests/test_save_to_sent.py b/tests/test_save_to_sent.py
@@ -414,3 +414,256 @@ async def test_send_email_returns_message(self, email_client):
             assert result is not None
             assert result["Subject"] == "Test Subject"
             assert "recipient@example.com" in result["To"]
+
+
+class TestFindSentFolderByFlag:
+    """Tests for _find_sent_folder_by_flag method."""
+
+    @pytest.fixture
+    def email_client(self):
+        """Create an EmailClient for testing."""
+        server = EmailServer(
+            user_name="test_user",
+            password="test_password",
+            host="smtp.example.com",
+            port=465,
+            use_ssl=True,
+        )
+        return EmailClient(server)
+
+    @pytest.fixture
+    def mock_imap_with_sent_flag(self):
+        """Create a mock IMAP client with Sent folder."""
+        mock = AsyncMock()
+        # Simulate IMAP LIST response with \Sent flag
+        mock.list = AsyncMock(
+            return_value=(
+                "OK",
+                [
+                    b'(\\HasNoChildren) "/" "INBOX"',
+                    b'(\\Sent \\HasNoChildren) "/" "Gesendete Objekte"',
+                    b'(\\Drafts \\HasNoChildren) "/" "Entwuerfe"',
+                ],
+            )
+        )
+        return mock
+
+    @pytest.fixture
+    def mock_imap_without_sent_flag(self):
+        """Create a mock IMAP client without Sent folder."""
+        mock = AsyncMock()
+        mock.list = AsyncMock(
+            return_value=(
+                "OK",
+                [
+                    b'(\\HasNoChildren) "/" "INBOX"',
+                    b'(\\Drafts \\HasNoChildren) "/" "Drafts"',
+                ],
+            )
+        )
+        return mock
+
+    @pytest.fixture
+    def mock_imap_list_error(self):
+        """Create a mock IMAP client that raises error on list."""
+        mock = AsyncMock()
+        mock.list = AsyncMock(side_effect=Exception("IMAP list failed"))
+        return mock
+
+    @pytest.mark.asyncio
+    async def test_find_sent_folder_by_flag_success(self, email_client, mock_imap_with_sent_flag):
+        """Test successful detection of Sent folder via IMAP flag."""
+        result = await email_client._find_sent_folder_by_flag(mock_imap_with_sent_flag)
+
+        assert result == "Gesendete Objekte"
+        mock_imap_with_sent_flag.list.assert_called_once_with('""', "*")
+
+    @pytest.mark.asyncio
+    async def test_find_sent_folder_by_flag_with_spaces(self, email_client):
+        """Test detection with folder name containing spaces (IONOS case)."""
+        mock = AsyncMock()
+        mock.list = AsyncMock(
+            return_value=(
+                "OK",
+                [b'(\\Sent \\HasNoChildren) "/" "Gesendete Objekte"'],
+            )
+        )
+
+        result = await email_client._find_sent_folder_by_flag(mock)
+        assert result == "Gesendete Objekte"
+
+    @pytest.mark.asyncio
+    async def test_find_sent_folder_by_flag_gmail_style(self, email_client):
+        """Test detection with Gmail-style folder structure."""
+        mock = AsyncMock()
+        mock.list = AsyncMock(
+            return_value=(
+                "OK",
+                [b'(\\Sent \\HasNoChildren) "/" "[Gmail]/Gesendet"'],
+            )
+        )
+
+        result = await email_client._find_sent_folder_by_flag(mock)
+        assert result == "[Gmail]/Gesendet"
+
+    @pytest.mark.asyncio
+    async def test_find_sent_folder_by_flag_not_found(self, email_client, mock_imap_without_sent_flag):
+        """Test when no folder with \\Sent flag exists."""
+        result = await email_client._find_sent_folder_by_flag(mock_imap_without_sent_flag)
+
+        assert result is None
+
+    @pytest.mark.asyncio
+    async def test_find_sent_folder_by_flag_handles_error(self, email_client, mock_imap_list_error):
+        """Test error handling when IMAP list fails."""
+        result = await email_client._find_sent_folder_by_flag(mock_imap_list_error)
+
+        assert result is None
+
+    @pytest.mark.asyncio
+    async def test_find_sent_folder_by_flag_with_string_response(self, email_client):
+        """Test handling of string (non-bytes) folder responses."""
+        mock = AsyncMock()
+        mock.list = AsyncMock(
+            return_value=(
+                "OK",
+                ['(\\Sent \\HasNoChildren) "/" "Sent Items"'],
+            )
+        )
+
+        result = await email_client._find_sent_folder_by_flag(mock)
+        assert result == "Sent Items"
+
+
+class TestAppendToSentWithFlagDetection:
+    """Tests for append_to_sent integration with flag detection."""
+
+    @pytest.fixture
+    def email_client(self):
+        """Create an EmailClient for testing."""
+        server = EmailServer(
+            user_name="test_user",
+            password="test_password",
+            host="smtp.example.com",
+            port=465,
+            use_ssl=True,
+        )
+        return EmailClient(server)
+
+    @pytest.fixture
+    def incoming_server(self):
+        """Create an incoming EmailServer for testing."""
+        return EmailServer(
+            user_name="test_user",
+            password="test_password",
+            host="imap.example.com",
+            port=993,
+            use_ssl=True,
+        )
+
+    @pytest.mark.asyncio
+    async def test_append_uses_flag_detected_folder(self, email_client, incoming_server):
+        """Test that append_to_sent uses flag-detected folder when auto-detecting."""
+        msg = MIMEText("Test body")
+
+        mock_imap = AsyncMock()
+        mock_imap._client_task = asyncio.Future()
+        mock_imap._client_task.set_result(None)
+        mock_imap.wait_hello_from_server = AsyncMock()
+        mock_imap.login = AsyncMock()
+        mock_imap.list = AsyncMock(
+            return_value=(
+                "OK",
+                [b'(\\Sent \\HasNoChildren) "/" "Gesendete Objekte"'],
+            )
+        )
+        mock_imap.select = AsyncMock(return_value=("OK", []))
+        mock_imap.append = AsyncMock(return_value=("OK", []))
+        mock_imap.logout = AsyncMock()
+
+        with patch("mcp_email_server.emails.classic.aioimaplib") as mock_aioimaplib:
+            mock_aioimaplib.IMAP4_SSL.return_value = mock_imap
+
+            result = await email_client.append_to_sent(msg, incoming_server, None)
+
+            assert result is True
+            # Verify it used the flag-detected folder with proper quoting
+            mock_imap.select.assert_called_with('"Gesendete Objekte"')
+
+    @pytest.mark.asyncio
+    async def test_append_prefers_flag_over_explicit(self, email_client, incoming_server):
+        """Test that IMAP flag detection has highest priority, even with explicit folder."""
+        msg = MIMEText("Test body")
+
+        mock_imap = AsyncMock()
+        mock_imap._client_task = asyncio.Future()
+        mock_imap._client_task.set_result(None)
+        mock_imap.wait_hello_from_server = AsyncMock()
+        mock_imap.login = AsyncMock()
+        mock_imap.list = AsyncMock(
+            return_value=(
+                "OK",
+                [b'(\\Sent \\HasNoChildren) "/" "Flag Detected"'],
+            )
+        )
+        mock_imap.select = AsyncMock(return_value=("OK", []))
+        mock_imap.append = AsyncMock(return_value=("OK", []))
+        mock_imap.logout = AsyncMock()
+
+        with patch("mcp_email_server.emails.classic.aioimaplib") as mock_aioimaplib:
+            mock_aioimaplib.IMAP4_SSL.return_value = mock_imap
+
+            # Even with explicit folder, flag-detected should be preferred (most reliable)
+            result = await email_client.append_to_sent(msg, incoming_server, "INBOX.Sent")
+
+            assert result is True
+            # Should use flag-detected folder (highest priority)
+            mock_imap.select.assert_called_with('"Flag Detected"')
+
+
+class TestHandlerErrorHandling:
+    """Tests for error handling in ClassicEmailHandler.send_email."""
+
+    @pytest.mark.asyncio
+    async def test_send_email_continues_on_append_error(self):
+        """Test that send_email continues even if append_to_sent fails."""
+        settings = EmailSettings(
+            account_name="test",
+            full_name="Test User",
+            email_address="test@example.com",
+            incoming=EmailServer(
+                user_name="test_user",
+                password="test_password",
+                host="imap.example.com",
+                port=993,
+                use_ssl=True,
+            ),
+            outgoing=EmailServer(
+                user_name="test_user",
+                password="test_password",
+                host="smtp.example.com",
+                port=465,
+                use_ssl=True,
+            ),
+            save_to_sent=True,
+            sent_folder_name="Sent",
+        )
+        handler = ClassicEmailHandler(settings)
+
+        mock_msg = MIMEText("Test body")
+        mock_send = AsyncMock(return_value=mock_msg)
+        mock_append = AsyncMock(side_effect=Exception("IMAP connection failed"))
+
+        with patch.object(handler.outgoing_client, "send_email", mock_send):
+            with patch.object(handler.outgoing_client, "append_to_sent", mock_append):
+                # Should not raise exception even though append fails
+                await handler.send_email(
+                    recipients=["recipient@example.com"],
+                    subject="Test",
+                    body="Test body",
+                )
+
+                # Email should still be sent
+                mock_send.assert_called_once()
+                # Append was attempted
+                mock_append.assert_called_once()
