From 15042c1b1bd382751fb391e2e36b19325bd729cd Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 20 Jun 2026 13:44:28 +0200 Subject: [PATCH] polish(mail): consistency guards (PR #935 round-7) Non-blocking consistency fixes: - scanner.py: skip re-queuing a mail_message whose placeholder status is "failed" (mirrors the file scanner's permanent-failure guard); the modified_at branch still retries once the message changes. - search/context.py: return None on an empty get_message payload during context expansion, mirroring the processor's index-time empty-payload guard. - tests: cover the non-numeric mailbox_id fail-open branch in the verifier. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/search/context.py | 4 ++++ nextcloud_mcp_server/vector/scanner.py | 10 ++++++++++ tests/unit/search/test_verification.py | 16 ++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/nextcloud_mcp_server/search/context.py b/nextcloud_mcp_server/search/context.py index e57c6602..d4219eea 100644 --- a/nextcloud_mcp_server/search/context.py +++ b/nextcloud_mcp_server/search/context.py @@ -834,6 +834,10 @@ async def _fetch_document_text( # Reconstruct full content via the shared helper so chunk offsets # match what the processor indexed (single source of truth). message = await nc_client.mail.get_message(int(doc_id)) + # Empty payload (OCS data=null with a <400 meta) -> skip context + # expansion, mirroring the processor's index-time guard. + if not message: + return None return build_mail_content(message) else: logger.warning("Unsupported doc_type for context expansion: %s", doc_type) diff --git a/nextcloud_mcp_server/vector/scanner.py b/nextcloud_mcp_server/vector/scanner.py index b4b0eea9..0468dc98 100644 --- a/nextcloud_mcp_server/vector/scanner.py +++ b/nextcloud_mcp_server/vector/scanner.py @@ -1549,6 +1549,16 @@ async def scan_mail_messages( needs_indexing = True elif existing_metadata.get("modified_at", 0) < modified_at: needs_indexing = True + elif existing_metadata.get("status") == "failed": + # A permanent processing failure — don't re-queue an + # unchanged message that will just fail again; the + # modified_at branch above retries once it changes + # (mirrors the file scanner's failed-placeholder guard). + logger.debug( + "Skipping mail message %s: previous processing " + "failed permanently", + doc_id, + ) elif existing_metadata.get("is_placeholder", False): queued_at = existing_metadata.get("queued_at", 0) placeholder_age = time.time() - queued_at diff --git a/tests/unit/search/test_verification.py b/tests/unit/search/test_verification.py index 9edbea24..5fbb4338 100644 --- a/tests/unit/search/test_verification.py +++ b/tests/unit/search/test_verification.py @@ -303,6 +303,22 @@ async def test_verify_mail_missing_mailbox_id_keeps(mocker): list_messages.assert_not_awaited() +@pytest.mark.unit +async def test_verify_mail_non_numeric_mailbox_id_keeps(mocker): + """A non-numeric mailbox_id in metadata is kept without a list call.""" + list_messages = mocker.AsyncMock() + mail_client = SimpleNamespace(list_messages=list_messages) + client = SimpleNamespace(mail=mail_client, username="alice") + + result = await _verify_mail_messages( + client, + [_make_result(42, doc_type="mail_message", metadata={"mailbox_id": "bad"})], + _sem(), + ) + assert result == {"42"} + list_messages.assert_not_awaited() + + @pytest.mark.unit async def test_verify_mail_non_numeric_id_kept_when_mailbox_listed(mocker): """A malformed doc_id can't match the numeric listing, so it's kept."""