From 10e768e4140576b0895120148d41528384acf61d Mon Sep 17 00:00:00 2001
From: Chris Coutinho <chris@coutinho.io>
Date: Sat, 20 Jun 2026 13:35:49 +0200
Subject: [PATCH] fix(mail): guard empty message payload in processor (PR #935
 round-6)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- processor.py: raise on an empty mail_message payload (OCS data=null with a
  <400 meta) so the task dead-letters instead of indexing a near-empty
  placeholder — mirrors the nc_mail_get_message tool guard. (the round-6
  approve-gating item)
- server/mail.py: clamp limit once in nc_mail_list_messages and base has_more on
  the effective (post-clamp) limit, so a caller passing limit<=0 doesn't get a
  misleading count.
- server/mail.py: note in nc_mail_get_message that attachments with id=null are
  inline body parts and can't be fetched via nc_mail_get_attachment.
- tests: add the first-time-missing incremental scanner case (enters the grace
  period, nothing queued/deleted).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 nextcloud_mcp_server/server/mail.py      | 13 +++++++++++--
 nextcloud_mcp_server/vector/processor.py |  7 +++++++
 tests/unit/vector/test_scanner_mail.py   | 21 +++++++++++++++++++++
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/nextcloud_mcp_server/server/mail.py b/nextcloud_mcp_server/server/mail.py
index b9ca5f9b..f0abdb3f 100644
--- a/nextcloud_mcp_server/server/mail.py
+++ b/nextcloud_mcp_server/server/mail.py
@@ -143,15 +143,22 @@ def configure_mail_tools(mcp: FastMCP):
             messages; page with ``cursor`` and stop on an empty result.
         """
         client = await get_client(ctx)
+        # Clamp to the same window the client/OCS API enforce so the has_more
+        # heuristic compares against the limit actually applied (a caller passing
+        # limit<=0 otherwise gets a misleading count).
+        effective_limit = min(max(1, limit), 100)
         try:
             messages_data = await client.mail.list_messages(
-                mailbox_id, cursor=cursor, search_filter=search_filter, limit=limit
+                mailbox_id,
+                cursor=cursor,
+                search_filter=search_filter,
+                limit=effective_limit,
             )
             messages = [MailMessageSummary(**m) for m in messages_data]
             return ListMessagesResponse(
                 results=messages,
                 total_count=len(messages),
-                has_more=len(messages) == limit and limit > 0,
+                has_more=len(messages) == effective_limit,
             )
         except RequestError as e:
             raise McpError(
@@ -181,6 +188,8 @@ def configure_mail_tools(mcp: FastMCP):
 
         Returns:
             GetMessageResponse with the full message including body and attachments.
+            Attachments with ``id: null`` are inline body parts and cannot be
+            fetched via nc_mail_get_attachment (which requires a string id).
         """
         client = await get_client(ctx)
         try:
diff --git a/nextcloud_mcp_server/vector/processor.py b/nextcloud_mcp_server/vector/processor.py
index 5b2ccf2c..d0486b6f 100644
--- a/nextcloud_mcp_server/vector/processor.py
+++ b/nextcloud_mcp_server/vector/processor.py
@@ -855,6 +855,13 @@ async def _index_document(
             if not is_valid_nextcloud_doc_id(doc_task.doc_id):
                 raise ValueError(f"Invalid mail_message doc_id: {doc_task.doc_id!r}")
             message = await nc_client.mail.get_message(int(doc_task.doc_id))
+            # An empty payload (OCS data=null with a <400 meta) would otherwise
+            # index a useless near-empty placeholder; fail loudly so the task
+            # dead-letters instead of corrupting the index.
+            if not message:
+                raise ValueError(
+                    f"mail_message {doc_task.doc_id!r} returned an empty payload"
+                )
             content = build_mail_content(message)
 
             subject = message.get("subject") or ""
diff --git a/tests/unit/vector/test_scanner_mail.py b/tests/unit/vector/test_scanner_mail.py
index c4cfe39e..df1c1031 100644
--- a/tests/unit/vector/test_scanner_mail.py
+++ b/tests/unit/vector/test_scanner_mail.py
@@ -229,3 +229,24 @@ async def test_incremental_deletes_after_grace_period(mocker):
     assert queued == 1
     assert [(t.doc_id, t.operation) for t in stream.tasks] == [("999", "delete")]
     assert ("alice", "999") not in scanner_module._potentially_deleted
+
+
+async def test_incremental_first_missing_starts_grace(mocker):
+    """A newly-missing indexed message enters the grace period (no delete yet)."""
+    _patch_incremental(mocker, indexed_ids=["999"], existing_metadata=None)
+    # Not previously seen as missing, and the mailbox now returns no messages.
+    nc_client = _single_message_client([])
+
+    stream = _CollectingStream()
+    queued = await scan_mail_messages(
+        user_id="alice",
+        send_stream=stream,
+        nc_client=nc_client,
+        initial_sync=False,
+        scan_id=1,
+    )
+
+    # First miss only starts the grace period — nothing queued, nothing deleted.
+    assert queued == 0
+    assert stream.tasks == []
+    assert ("alice", "999") in scanner_module._potentially_deleted