From f738081f4b6322ea2878aa647717beaafc4265ab Mon Sep 17 00:00:00 2001 From: Oleksandr Piskun Date: Tue, 15 Sep 2026 17:10:47 +0000 Subject: [PATCH 1/2] Add Mail tools to move, flag and tag messages Five write tools call the Mail app's JSON routes through a new app_request_json client helper, and list/get output now includes tags. The Mail integration tests read from Dovecot instead of smtp4dev, whose IMAP server has no MOVE, folders or keywords. Closes #59. Signed-off-by: Oleksandr Piskun --- .github/workflows/tests-integration.yml | 22 +- PROGRESS.md | 9 +- README.md | 11 +- src/nc_mcp_server/client.py | 60 ++++- src/nc_mcp_server/tools/mail.py | 218 ++++++++++++++++- tests/integration/test_mail.py | 303 ++++++++++++++++++++++-- tests/test_client_app_request.py | 130 ++++++++++ tests/test_mail_tools.py | 197 +++++++++++++++ 8 files changed, 909 insertions(+), 41 deletions(-) create mode 100644 tests/test_client_app_request.py create mode 100644 tests/test_mail_tools.py diff --git a/.github/workflows/tests-integration.yml b/.github/workflows/tests-integration.yml index 5b1fe82..b70eb5f 100644 --- a/.github/workflows/tests-integration.yml +++ b/.github/workflows/tests-integration.yml @@ -31,14 +31,20 @@ jobs: --health-timeout 5s --health-retries 30 --health-start-period 60s + # The Mail account sends through smtp4dev and reads from Dovecot. smtp4dev's own IMAP server has + # no MOVE, no folders and drops \Flagged and keywords, so it can't back the move/flag/tag tools. smtp4dev: image: rnwood/smtp4dev:latest env: ServerOptions__BasePath: /smtp4dev ports: - - 9025:25 - - 9143:143 - 9080:80 + dovecot: + image: dovecot/dovecot:2.4.5 + env: + USER_PASSWORD: test + ports: + - 9143:31143 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -79,9 +85,12 @@ jobs: $OCC "php occ app:install forms" || echo "forms already installed" $OCC "php occ app:install cospend" || echo "cospend already installed" $OCC "php occ app:enable circles" || echo "circles enable failed (may not be shipped)" + # Dovecot refuses plaintext logins, and its image ships a self-signed certificate for STARTTLS. + $OCC "php occ config:system:set app.mail.verify-tls-peer --value=false --type=boolean" SMTP4DEV_IP=$(docker inspect ${{ job.services.smtp4dev.id }} --format '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}') - echo "smtp4dev IP: $SMTP4DEV_IP" - $OCC "php occ mail:account:create admin 'Test Mail' test@localhost $SMTP4DEV_IP 143 none test test $SMTP4DEV_IP 25 none test test" + DOVECOT_IP=$(docker inspect ${{ job.services.dovecot.id }} --format '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}') + echo "smtp4dev IP: $SMTP4DEV_IP, dovecot IP: $DOVECOT_IP" + $OCC "php occ mail:account:create admin 'Test Mail' test@localhost $DOVECOT_IP 31143 tls test test $SMTP4DEV_IP 25 none test test" $OCC "php occ mail:account:sync 1" APP_PASS=$($OCC "php occ user:auth-tokens:add admin" | grep -oP '[A-Za-z0-9]{72}' | head -1) if [ -z "$APP_PASS" ]; then echo "::error::Failed to generate app password"; exit 1; fi @@ -99,7 +108,10 @@ jobs: NEXTCLOUD_MCP_APP_PASSWORD: "true" SMTP4DEV_HOST: localhost SMTP4DEV_HTTP_PORT: "9080" - SMTP4DEV_SMTP_PORT: "9025" + MAIL_IMAP_HOST: localhost + MAIL_IMAP_PORT: "9143" + MAIL_IMAP_USER: test + MAIL_IMAP_PASSWORD: test MAIL_RECIPIENT: test@localhost NC_CONTAINER: ${{ job.services.nextcloud.id }} MAIL_ACCOUNT_ID: "1" diff --git a/PROGRESS.md b/PROGRESS.md index 8a3b5db..8b2b314 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -43,6 +43,7 @@ - [x] Forms tools: 25 tools covering forms, questions, options, shares, submissions CRUD + export (2026-04-23) - [x] Circles (Teams) tools: 14 tools — list/CRUD circles, member add/remove/level, search, join/leave (2026-04-24) - [x] Cospend tools: 16 tools — projects (7), members (4), bills (5) — shared expense tracking (2026-04-26) +- [x] Mail triage tools: move_mail_message, set_mail_message_flags, create_mail_tag, add_mail_message_tag, remove_mail_message_tag via the Mail app's JSON routes; tags in message output; Mail integration tests read from Dovecot (2026-09-15) ### In Progress @@ -81,7 +82,7 @@ | Versions | 2 | 18 | | Shares | 5 | 40 | | System Tags | 6 | 22 | -| Mail | 5 | 29 | +| Mail | 10 | 70 | | Collectives | 3 | 22 | | App Management | 4 | 14 | | Calendar | 6 | 44 | @@ -92,7 +93,7 @@ | Server | — | 8 | | Permissions | — | 34 | | Errors | — | 16 | -| Client | — | 29 | +| Client | — | 45 | | Config | — | 24 | | State | — | 2 | | File Helpers | — | 26 | @@ -100,7 +101,7 @@ | Forms | 25 | 34 | | Circles | 14 | 31 | | Cospend | 16 | 35 | -| **Total** | **157** | **871** | +| **Total** | **162** | **928** | Files shows 10, but one (`upload_file_from_path`) is only registered when -`NEXTCLOUD_MCP_UPLOAD_ROOT` is configured. Default deployments expose 156 tools. +`NEXTCLOUD_MCP_UPLOAD_ROOT` is configured. Default deployments expose 161 tools. diff --git a/README.md b/README.md index 33d236f..77d2513 100644 --- a/README.md +++ b/README.md @@ -30,9 +30,9 @@ export NEXTCLOUD_PASSWORD=your-app-password nc-mcp-server ``` -## 140 Tools Across 23 Nextcloud Apps +## 161 Tools Across 23 Nextcloud Apps -A 141st tool, `upload_file_from_path`, is registered only when the operator sets +A 162nd tool, `upload_file_from_path`, is registered only when the operator sets `NEXTCLOUD_MCP_UPLOAD_ROOT`. See [Files](#files) for details. | Category | Tools | Protocol | @@ -54,7 +54,7 @@ A 141st tool, `upload_file_from_path`, is registered only when the operator sets | [Calendar](#calendar) | list calendars, CRUD events | CalDAV | | [Contacts](#contacts) | list address books, CRUD contacts | CardDAV | | [Tasks](#tasks) | list lists, CRUD tasks, complete | CalDAV | -| [Mail](#mail) | accounts, mailboxes, messages, send | OCS | +| [Mail](#mail) | accounts, mailboxes, messages, send, move, flags, tags | OCS + REST | | [Collectives](#collectives) | list, pages, create, trash, restore | OCS | | [Forms](#forms) | CRUD forms, questions, options, shares, submissions + export | OCS | | [Circles (Teams)](#circles-teams) | list, CRUD, members (add/remove/promote), join/leave, search | OCS | @@ -354,6 +354,11 @@ call; the body is streamed in chunks rather than loaded into memory. | `list_mail_messages` | read | List messages in a mailbox | | `get_mail_message` | read | Get full message content | | `send_mail` | write | Send an email | +| `move_mail_message` | write | Move a message to another mailbox (its ID changes) | +| `set_mail_message_flags` | write | Mark as read/unread, starred, answered | +| `create_mail_tag` | write | Create a tag, or get the existing one with the same label | +| `add_mail_message_tag` | write | Tag a message | +| `remove_mail_message_tag` | write | Remove a tag from a message | ### Collectives diff --git a/src/nc_mcp_server/client.py b/src/nc_mcp_server/client.py index dcd6d2a..231727c 100644 --- a/src/nc_mcp_server/client.py +++ b/src/nc_mcp_server/client.py @@ -4,7 +4,7 @@ import logging import xml.etree.ElementTree as ET from collections.abc import AsyncIterable, Callable -from typing import Any +from typing import Any, cast from urllib.parse import quote as url_quote import niquests @@ -66,6 +66,36 @@ def _raise_for_ocs_status(response: niquests.Response, context: str = "") -> Non raise NextcloudError(f"{prefix}{detail}", code) +def _app_error_message(body: object) -> str: + """Return the error message from an app route's JSON body, or "" when it has none. + + App routes outside OCS have no common error envelope. The Mail app, for example, answers + client errors with {"status": "fail", "data": {"message": ...}} and server errors with + {"status": "error", "message": ...}. + """ + if not isinstance(body, dict): + return "" + payload = cast(dict[str, Any], body) + data = payload.get("data") + message = cast(dict[str, Any], data).get("message") if isinstance(data, dict) else None + if message is None: + message = payload.get("message") + return message if isinstance(message, str) else "" + + +def _raise_for_app_status(response: niquests.Response, context: str = "") -> None: + """Raise NextcloudError for a failed app route, preferring the message from its JSON body.""" + if response.ok: + return + code = response.status_code or 0 + prefix = f"{context}: " if context else "" + message = "" + with contextlib.suppress(ValueError, TypeError): + message = _app_error_message(response.json()) + detail = message or _STATUS_MESSAGES.get(code, f"HTTP {code}") + raise NextcloudError(f"{prefix}{detail}", code) + + # XML namespaces used in WebDAV responses DAV_NS = "DAV:" OC_NS = "http://owncloud.org/ns" @@ -287,6 +317,34 @@ async def ocs_put_json(self, path: str, json_data: dict[str, Any] | None = None) result: dict[str, Any] = response.json() # type: ignore[assignment] return result["ocs"]["data"] + # --- App JSON routes --- + + async def app_request_json( + self, + method: str, + path: str, + json_data: dict[str, Any] | None = None, + params: dict[str, Any] | None = None, + ) -> Any: + """Call an app's non-OCS JSON route under /index.php/apps/ and return the decoded body. + + Some app actions exist only as the routes an app's web UI uses (Mail's move, flag and tag + endpoints, for example). Those controllers require a CSRF token, and Nextcloud skips that + check for requests carrying ``OCS-APIRequest: true``, so the header is sent explicitly. + Responses are plain JSON without an OCS envelope; an empty body returns None. + """ + url = f"{self._base_url}/index.php/apps/{path}" + kwargs: dict[str, Any] = {"headers": {"OCS-APIRequest": "true", "Accept": "application/json"}} + if json_data is not None: + kwargs["json"] = json_data + if params: + kwargs["params"] = params + response = await self._do_request(method, url, **kwargs) + _raise_for_app_status(response, f"{method} apps/{path}") + if not (response.content or b"").strip(): + return None + return response.json() + # --- WebDAV --- async def dav_propfind(self, path: str, depth: int = 1) -> list[dict[str, Any]]: diff --git a/src/nc_mcp_server/tools/mail.py b/src/nc_mcp_server/tools/mail.py index 392deab..1390ed0 100644 --- a/src/nc_mcp_server/tools/mail.py +++ b/src/nc_mcp_server/tools/mail.py @@ -1,15 +1,22 @@ -"""Mail tools — list accounts, mailboxes, messages; get message details; send email via OCS API.""" +"""Mail tools - accounts, mailboxes, messages, send, move, flags and tags via the Mail app's OCS and JSON APIs.""" import json -from typing import Any +import re +from typing import Any, cast +from urllib.parse import quote from mcp.server.fastmcp import FastMCP -from ..annotations import ADDITIVE, READONLY +from ..annotations import ADDITIVE, ADDITIVE_IDEMPOTENT, READONLY +from ..client import NextcloudError from ..permissions import PermissionLevel, require_permission from ..state import get_client MAIL_OCS = "apps/mail" +# Moving, flagging and tagging have no OCS endpoints, only the Mail web UI's JSON routes. +MAIL_API = "mail/api" +MAX_TAG_NAME_LENGTH = 128 +TAG_COLOR_RE = re.compile(r"^#(?:[0-9a-fA-F]{3}|[0-9a-fA-F]{6})$") def _format_account(account: dict[str, Any]) -> dict[str, Any]: @@ -31,6 +38,31 @@ def _format_mailbox(mailbox: dict[str, Any]) -> dict[str, Any]: } +def _format_tag(tag: dict[str, Any]) -> dict[str, Any]: + return { + "id": tag.get("id"), + "display_name": tag.get("displayName"), + "imap_label": tag.get("imapLabel"), + "color": tag.get("color"), + } + + +def _format_tags(tags: object) -> list[dict[str, Any]]: + """Format message tags, which Mail serializes as an object keyed by IMAP label, or [] when there are none.""" + if isinstance(tags, dict): + items: list[object] = list(cast(dict[str, object], tags).values()) + elif isinstance(tags, list): + items = cast(list[object], tags) + else: + return [] + result: list[dict[str, Any]] = [] + for item in items: + if isinstance(item, dict): + tag = cast(dict[str, Any], item) + result.append({"display_name": tag.get("displayName"), "imap_label": tag.get("imapLabel")}) + return result + + def _format_message_summary(msg: dict[str, Any]) -> dict[str, Any]: result: dict[str, Any] = { "id": msg.get("databaseId"), @@ -45,6 +77,9 @@ def _format_message_summary(msg: dict[str, Any]) -> dict[str, Any]: active_flags = [k for k, v in flags.items() if v and k != "$notjunk"] if active_flags: result["flags"] = active_flags + tags = _format_tags(msg.get("tags")) + if tags: + result["tags"] = tags if msg.get("cc"): result["cc"] = msg["cc"] preview = msg.get("previewText") @@ -85,6 +120,31 @@ def _format_message_full(msg: dict[str, Any]) -> dict[str, Any]: return result +async def _mail_api(method: str, path: str, forbidden: str, json_data: dict[str, Any] | None = None) -> Any: + """Call a Mail JSON route, replacing Mail's bare 403 with a message that says what was not found. + + Mail answers 403 with an empty body for message IDs that do not exist or belong to someone else, + and for tag labels the user has no tag for. + """ + try: + return await get_client().app_request_json(method, f"{MAIL_API}/{path}", json_data=json_data) + except NextcloudError as e: + if e.status_code == 403: + raise NextcloudError(forbidden, 403) from e + raise + + +def _message_not_found(message_id: int) -> str: + return f"Message {message_id} was not found or is not accessible." + + +def _tag_not_found(message_id: int, imap_label: str) -> str: + return ( + f"Message {message_id} was not found, or you have no tag with IMAP label '{imap_label}'. " + "Use create_mail_tag to create the tag and get its label." + ) + + def _register_read_tools(mcp: FastMCP) -> None: @mcp.tool(annotations=READONLY) @require_permission(PermissionLevel.READ) @@ -126,7 +186,7 @@ async def list_mailboxes(account_id: int) -> str: async def list_mail_messages(mailbox_id: int, limit: int = 20, cursor: int | None = None) -> str: """List messages in a mailbox, newest first. - Returns message summaries (subject, sender, date, flags) without the full body. + Returns message summaries (subject, sender, date, flags, tags) without the full body. Use get_mail_message with a message ID to read the full content. Args: @@ -137,7 +197,8 @@ async def list_mail_messages(mailbox_id: int, limit: int = 20, cursor: int | Non Returns: JSON object with "data" (list of message summaries) and "pagination" metadata. - Each message has: id, subject, date (unix timestamp), from, to, flags, preview. + Each message has: id, subject, date (unix timestamp), from, to, flags, preview, + and tags (display_name and imap_label) when it has any. """ client = get_client() limit = max(1, min(100, limit)) @@ -160,18 +221,24 @@ async def list_mail_messages(mailbox_id: int, limit: int = 20, cursor: int | Non async def get_mail_message(message_id: int) -> str: """Get a full email message including its body. - Retrieves the complete message with body text, message ID, and attachment metadata. + Retrieves the complete message with body text, message ID, tags, and attachment metadata. Args: message_id: The message database ID. Use list_mail_messages to find it. Returns: JSON object with: id, subject, date, from, to, cc, bcc, message_id, - body, flags, and attachments list (if any). + body, flags, tags (display_name and imap_label, if any), and attachments list (if any). """ client = get_client() data = await client.ocs_get(f"{MAIL_OCS}/message/{message_id}") - return json.dumps(_format_message_full(data)) + result = _format_message_full(data) + # The OCS message endpoint leaves tags out; the Mail web UI's message route includes them. + details = await client.app_request_json("GET", f"{MAIL_API}/messages/{message_id}") + tags = _format_tags(details.get("tags") if details else None) + if tags: + result["tags"] = tags + return json.dumps(result) def _register_write_tools(mcp: FastMCP) -> None: @@ -227,7 +294,142 @@ async def send_mail( return f"Email sent to {to_str}." +def _register_triage_tools(mcp: FastMCP) -> None: + @mcp.tool(annotations=ADDITIVE) + @require_permission(PermissionLevel.WRITE) + async def move_mail_message(message_id: int, destination_mailbox_id: int) -> str: + """Move a message to another mailbox (folder), in the same or another mail account. + + The moved message gets a new ID, so message_id is no longer valid afterwards and must not + be reused. IMAP UIDs are per mailbox, and Nextcloud assigns the new ID when it syncs the + destination mailbox. The inbox and mailboxes with background sync enabled are synced + automatically; other folders are synced when opened in the Mail app. + + Args: + message_id: The message database ID. Use list_mail_messages to find it. + destination_mailbox_id: The target mailbox ID. Use list_mailboxes to find it. + + Returns: + Confirmation message on success. + """ + await _mail_api( + "POST", + f"messages/{message_id}/move", + f"Message {message_id} or mailbox {destination_mailbox_id} was not found or is not accessible.", + json_data={"destFolderId": destination_mailbox_id}, + ) + return ( + f"Message {message_id} moved to mailbox {destination_mailbox_id}. " + "Its old ID is no longer valid; list the destination mailbox to find the message." + ) + + @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) + @require_permission(PermissionLevel.WRITE) + async def set_mail_message_flags( + message_id: int, + seen: bool | None = None, + flagged: bool | None = None, + answered: bool | None = None, + ) -> str: + """Set or clear flags on a message: read/unread, starred, answered. + + Only the flags you pass change; the others keep their current value. + + Args: + message_id: The message database ID. Use list_mail_messages to find it. + seen: True marks the message as read, false as unread. + flagged: True stars (flags) the message, false removes the star. + answered: True marks the message as answered, false clears that mark. + + Returns: + JSON object with message_id and the flags that were set. + """ + requested = {"seen": seen, "flagged": flagged, "answered": answered} + flags = {name: value for name, value in requested.items() if value is not None} + if not flags: + raise ValueError("Pass at least one of seen, flagged or answered.") + await _mail_api( + "PUT", f"messages/{message_id}/flags", _message_not_found(message_id), json_data={"flags": flags} + ) + return json.dumps({"message_id": message_id, "flags": flags}) + + +def _register_tag_tools(mcp: FastMCP) -> None: + @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) + @require_permission(PermissionLevel.WRITE) + async def create_mail_tag(display_name: str, color: str) -> str: + """Create a mail tag, or get the existing tag with the same IMAP label. + + Tags belong to the current user and are stored on messages as IMAP keywords. Mail derives + the IMAP label from the display name (for example "Needs Reply" becomes "$needs_reply"). + If a tag with that label already exists, it is returned unchanged, including its color. + + Args: + display_name: Tag name shown in the Mail app (at most 128 characters). + color: Hex color such as "#0082c9". + + Returns: + JSON object with id, display_name, imap_label and color. Pass imap_label to + add_mail_message_tag and remove_mail_message_tag. + """ + name = display_name.strip() + if not name: + raise ValueError("display_name must not be empty.") + if len(name) > MAX_TAG_NAME_LENGTH: + raise ValueError(f"display_name must be at most {MAX_TAG_NAME_LENGTH} characters.") + if not TAG_COLOR_RE.match(color): + raise ValueError(f"Invalid color '{color}'. Use a hex color such as '#0082c9'.") + client = get_client() + tag = await client.app_request_json("POST", f"{MAIL_API}/tags", json_data={"displayName": name, "color": color}) + return json.dumps(_format_tag(tag)) + + @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) + @require_permission(PermissionLevel.WRITE) + async def add_mail_message_tag(message_id: int, imap_label: str) -> str: + """Tag a message. Tagging a message that already has the tag changes nothing. + + Args: + message_id: The message database ID. Use list_mail_messages to find it. + imap_label: The tag's IMAP label (for example "$needs_reply"), as returned by + create_mail_tag or shown in the tags of list_mail_messages. + + Returns: + JSON object with message_id and the tag (id, display_name, imap_label, color). + """ + if not imap_label: + raise ValueError("imap_label must not be empty.") + tag = await _mail_api( + "PUT", + f"messages/{message_id}/tags/{quote(imap_label, safe='')}", + _tag_not_found(message_id, imap_label), + ) + return json.dumps({"message_id": message_id, "tag": _format_tag(tag)}) + + @mcp.tool(annotations=ADDITIVE_IDEMPOTENT) + @require_permission(PermissionLevel.WRITE) + async def remove_mail_message_tag(message_id: int, imap_label: str) -> str: + """Remove a tag from a message. The tag itself is kept and can still be used on other messages. + + Args: + message_id: The message database ID. Use list_mail_messages to find it. + imap_label: The tag's IMAP label (for example "$needs_reply"). + + Returns: + JSON object with message_id and the removed tag (id, display_name, imap_label, color). + """ + if not imap_label: + raise ValueError("imap_label must not be empty.") + tag = await _mail_api( + "DELETE", + f"messages/{message_id}/tags/{quote(imap_label, safe='')}", + _tag_not_found(message_id, imap_label), + ) + return json.dumps({"message_id": message_id, "tag": _format_tag(tag)}) + + def register(mcp: FastMCP) -> None: """Register mail tools with the MCP server.""" _register_read_tools(mcp) _register_write_tools(mcp) + _register_triage_tools(mcp) + _register_tag_tools(mcp) diff --git a/tests/integration/test_mail.py b/tests/integration/test_mail.py index 76209a4..8890732 100644 --- a/tests/integration/test_mail.py +++ b/tests/integration/test_mail.py @@ -1,12 +1,23 @@ -"""Integration tests for Mail tools against a real Nextcloud instance with smtp4dev.""" +"""Integration tests for Mail tools against a real Nextcloud instance. + +The test Mail account reads mail from Dovecot over IMAP (STARTTLS) and sends through smtp4dev. +Incoming test messages are put straight into Dovecot's INBOX with IMAP APPEND, and smtp4dev's +REST API shows what send_mail delivered. smtp4dev's own IMAP server can't be used for reading: +it has no MOVE, no folders, and drops \\Flagged and keywords (tags). +""" import asyncio +import contextlib +import email.utils +import imaplib import json import os -import smtplib +import ssl import subprocess import time import urllib.request +import uuid +from collections.abc import AsyncGenerator from email.mime.text import MIMEText from typing import Any @@ -20,10 +31,13 @@ SMTP4DEV_HOST = os.environ.get("SMTP4DEV_HOST", "smtp4dev.ncmcp") SMTP4DEV_HTTP_PORT = int(os.environ.get("SMTP4DEV_HTTP_PORT", "80")) SMTP4DEV_API = f"http://{SMTP4DEV_HOST}:{SMTP4DEV_HTTP_PORT}/smtp4dev/api" -SMTP_HOST = SMTP4DEV_HOST -SMTP_PORT = int(os.environ.get("SMTP4DEV_SMTP_PORT", "25")) -MAIL_RECIPIENT = os.environ.get("MAIL_RECIPIENT", f"test@{SMTP4DEV_HOST}") +IMAP_HOST = os.environ.get("MAIL_IMAP_HOST", "dovecot.ncmcp") +IMAP_PORT = int(os.environ.get("MAIL_IMAP_PORT", "31143")) +IMAP_USER = os.environ.get("MAIL_IMAP_USER", "test") +IMAP_PASSWORD = os.environ.get("MAIL_IMAP_PASSWORD", "test") +MAIL_RECIPIENT = os.environ.get("MAIL_RECIPIENT", "test@localhost") UNIQUE = "mcp-test-mail" +ARCHIVE_MAILBOX = "mcp-test-archive" def _smtp4dev_delete_all() -> None: @@ -38,14 +52,25 @@ def _smtp4dev_list_messages() -> list[dict[str, Any]]: return data.get("results", []) -def _send_test_email(subject: str, body: str = "test body", to: str = MAIL_RECIPIENT) -> None: - """Send a test email directly via SMTP to smtp4dev.""" +def _deliver_test_email(subject: str, body: str = "test body") -> None: + """Put a test message straight into the test account's INBOX with IMAP APPEND.""" msg = MIMEText(body) msg["Subject"] = subject msg["From"] = "external-sender@test.local" - msg["To"] = to - with smtplib.SMTP(SMTP_HOST, SMTP_PORT, timeout=10) as smtp: - smtp.sendmail("external-sender@test.local", [to], msg.as_string()) + msg["To"] = MAIL_RECIPIENT + msg["Date"] = email.utils.formatdate() + msg["Message-ID"] = email.utils.make_msgid(domain="test.local") + tls = ssl.create_default_context() + tls.check_hostname = False + tls.verify_mode = ssl.CERT_NONE + imap = imaplib.IMAP4(IMAP_HOST, IMAP_PORT, timeout=10) + try: + imap.starttls(ssl_context=tls) + imap.login(IMAP_USER, IMAP_PASSWORD) + imap.append("INBOX", None, imaplib.Time2Internaldate(time.time()), msg.as_bytes()) + finally: + with contextlib.suppress(Exception): + imap.logout() def _sync_mail_account(account_id: int) -> None: @@ -56,13 +81,20 @@ def _sync_mail_account(account_id: int) -> None: ["docker", "exec", container, "su", "-s", "/bin/bash", "www-data", "-c", cmd], capture_output=True, text=True, - timeout=30, + timeout=60, check=False, ) if result.returncode != 0: raise AssertionError(f"mail:account:sync {account_id} failed: {result.stderr}") +async def _sync_mailbox(nc_mcp: McpTestHelper, mailbox_id: int) -> None: + """Sync one mailbox; mail:account:sync skips folders that have no background sync.""" + await nc_mcp.client.app_request_json( + "POST", f"mail/api/mailboxes/{mailbox_id}/sync", json_data={"ids": [], "init": True} + ) + + async def _get_account_id(nc_mcp: McpTestHelper) -> int: """Get the test mail account ID.""" result = await nc_mcp.call("list_mail_accounts") @@ -88,6 +120,51 @@ async def _get_inbox_id(nc_mcp: McpTestHelper, account_id: int) -> int: return inbox["id"] +async def _find_message(nc_mcp: McpTestHelper, mailbox_id: int, subject: str) -> dict[str, Any] | None: + """Find a message by exact subject among the newest messages of a mailbox.""" + listing = json.loads(await nc_mcp.call("list_mail_messages", mailbox_id=mailbox_id, limit=20)) + return next((m for m in listing["data"] if m.get("subject") == subject), None) + + +async def _deliver_and_find(nc_mcp: McpTestHelper, label: str) -> tuple[int, int, dict[str, Any]]: + """Deliver a uniquely named message to INBOX, sync, and return (account_id, inbox_id, message).""" + account_id = await _get_account_id(nc_mcp) + inbox_id = await _get_inbox_id(nc_mcp, account_id) + subject = f"{UNIQUE}-{label}-{uuid.uuid4().hex[:8]}" + _deliver_test_email(subject) + _sync_mail_account(account_id) + message = await _find_message(nc_mcp, inbox_id, subject) + assert message is not None, f"Delivered message '{subject}' not found in INBOX" + return account_id, inbox_id, message + + +async def _delete_tag(nc_mcp: McpTestHelper, account_id: int, tag_id: int) -> None: + with contextlib.suppress(Exception): + await nc_mcp.client.app_request_json("DELETE", f"mail/api/tags/{account_id}/delete/{tag_id}") + + +def _tag_ref(tag: dict[str, Any]) -> dict[str, Any]: + return {"display_name": tag["display_name"], "imap_label": tag["imap_label"]} + + +@pytest.fixture +async def archive_mailbox(nc_mcp: McpTestHelper) -> AsyncGenerator[int]: + """A scratch mailbox to move messages into, deleted with its messages after the test.""" + account_id = await _get_account_id(nc_mcp) + mailboxes = json.loads(await nc_mcp.call("list_mailboxes", account_id=account_id)) + existing = next((mb for mb in mailboxes if mb["name"] == ARCHIVE_MAILBOX), None) + if existing is not None: + mailbox_id: int = existing["id"] + else: + created = await nc_mcp.client.app_request_json( + "POST", "mail/api/mailboxes", json_data={"accountId": account_id, "name": ARCHIVE_MAILBOX} + ) + mailbox_id = created["databaseId"] + yield mailbox_id + with contextlib.suppress(Exception): + await nc_mcp.client.app_request_json("DELETE", f"mail/api/mailboxes/{mailbox_id}") + + class TestListMailAccounts: @pytest.mark.asyncio async def test_returns_list(self, nc_mcp: McpTestHelper) -> None: @@ -111,7 +188,7 @@ async def test_account_email_matches(self, nc_mcp: McpTestHelper) -> None: result = await nc_mcp.call("list_mail_accounts") accounts = json.loads(result) emails = [a["email"] for a in accounts] - assert any("smtp4dev" in e or "test" in e for e in emails) + assert MAIL_RECIPIENT in emails class TestListMailboxes: @@ -174,7 +251,7 @@ async def test_returns_data_and_pagination(self, nc_mcp: McpTestHelper) -> None: async def test_messages_have_required_fields(self, nc_mcp: McpTestHelper) -> None: account_id = await _get_account_id(nc_mcp) inbox_id = await _get_inbox_id(nc_mcp, account_id) - _send_test_email(f"{UNIQUE}-fields") + _deliver_test_email(f"{UNIQUE}-fields") _sync_mail_account(account_id) result = await nc_mcp.call("list_mail_messages", mailbox_id=inbox_id) parsed = json.loads(result) @@ -192,7 +269,7 @@ async def test_limit_parameter(self, nc_mcp: McpTestHelper) -> None: account_id = await _get_account_id(nc_mcp) inbox_id = await _get_inbox_id(nc_mcp, account_id) for i in range(3): - _send_test_email(f"{UNIQUE}-limit-{i}") + _deliver_test_email(f"{UNIQUE}-limit-{i}") _sync_mail_account(account_id) result = await nc_mcp.call("list_mail_messages", mailbox_id=inbox_id, limit=2) parsed = json.loads(result) @@ -212,7 +289,7 @@ async def test_cursor_pagination(self, nc_mcp: McpTestHelper) -> None: account_id = await _get_account_id(nc_mcp) inbox_id = await _get_inbox_id(nc_mcp, account_id) for i in range(3): - _send_test_email(f"{UNIQUE}-cursor-{i}") + _deliver_test_email(f"{UNIQUE}-cursor-{i}") _sync_mail_account(account_id) first_page = json.loads(await nc_mcp.call("list_mail_messages", mailbox_id=inbox_id, limit=2)) if first_page["pagination"]["has_more"]: @@ -228,7 +305,7 @@ async def test_cursor_pagination(self, nc_mcp: McpTestHelper) -> None: async def test_message_from_field_structure(self, nc_mcp: McpTestHelper) -> None: account_id = await _get_account_id(nc_mcp) inbox_id = await _get_inbox_id(nc_mcp, account_id) - _send_test_email(f"{UNIQUE}-from-struct") + _deliver_test_email(f"{UNIQUE}-from-struct") _sync_mail_account(account_id) result = await nc_mcp.call("list_mail_messages", mailbox_id=inbox_id, limit=5) parsed = json.loads(result) @@ -249,7 +326,7 @@ class TestGetMailMessage: async def test_returns_full_message(self, nc_mcp: McpTestHelper) -> None: account_id = await _get_account_id(nc_mcp) inbox_id = await _get_inbox_id(nc_mcp, account_id) - _send_test_email(f"{UNIQUE}-get-full", body="Hello from integration test") + _deliver_test_email(f"{UNIQUE}-get-full", body="Hello from integration test") _sync_mail_account(account_id) messages = json.loads(await nc_mcp.call("list_mail_messages", mailbox_id=inbox_id, limit=5)) target = next((m for m in messages["data"] if UNIQUE in str(m.get("subject", ""))), None) @@ -266,7 +343,7 @@ async def test_returns_full_message(self, nc_mcp: McpTestHelper) -> None: async def test_body_contains_content(self, nc_mcp: McpTestHelper) -> None: account_id = await _get_account_id(nc_mcp) inbox_id = await _get_inbox_id(nc_mcp, account_id) - _send_test_email(f"{UNIQUE}-body-check", body="unique-body-content-12345") + _deliver_test_email(f"{UNIQUE}-body-check", body="unique-body-content-12345") _sync_mail_account(account_id) messages = json.loads(await nc_mcp.call("list_mail_messages", mailbox_id=inbox_id, limit=5)) target = next((m for m in messages["data"] if "body-check" in str(m.get("subject", ""))), None) @@ -280,7 +357,7 @@ async def test_subject_matches(self, nc_mcp: McpTestHelper) -> None: account_id = await _get_account_id(nc_mcp) inbox_id = await _get_inbox_id(nc_mcp, account_id) subject = f"{UNIQUE}-subject-match-{int(time.time())}" - _send_test_email(subject, body="test") + _deliver_test_email(subject, body="test") _sync_mail_account(account_id) messages = json.loads(await nc_mcp.call("list_mail_messages", mailbox_id=inbox_id, limit=5)) target = next((m for m in messages["data"] if subject in str(m.get("subject", ""))), None) @@ -293,7 +370,7 @@ async def test_subject_matches(self, nc_mcp: McpTestHelper) -> None: async def test_from_field(self, nc_mcp: McpTestHelper) -> None: account_id = await _get_account_id(nc_mcp) inbox_id = await _get_inbox_id(nc_mcp, account_id) - _send_test_email(f"{UNIQUE}-from-check") + _deliver_test_email(f"{UNIQUE}-from-check") _sync_mail_account(account_id) messages = json.loads(await nc_mcp.call("list_mail_messages", mailbox_id=inbox_id, limit=5)) target = next((m for m in messages["data"] if "from-check" in str(m.get("subject", ""))), None) @@ -380,6 +457,152 @@ async def test_send_nonexistent_account_raises(self, nc_mcp: McpTestHelper) -> N ) +class TestMoveMailMessage: + @pytest.mark.asyncio + async def test_move_to_other_mailbox(self, nc_mcp: McpTestHelper, archive_mailbox: int) -> None: + _, inbox_id, message = await _deliver_and_find(nc_mcp, "move") + result = await nc_mcp.call( + "move_mail_message", message_id=message["id"], destination_mailbox_id=archive_mailbox + ) + assert "no longer valid" in result + assert await _find_message(nc_mcp, inbox_id, message["subject"]) is None + await _sync_mailbox(nc_mcp, archive_mailbox) + moved = await _find_message(nc_mcp, archive_mailbox, message["subject"]) + assert moved is not None, "Moved message not found in the destination mailbox" + assert moved["id"] != message["id"] + assert moved["mailbox_id"] == archive_mailbox + full = json.loads(await nc_mcp.call("get_mail_message", message_id=moved["id"])) + assert full["subject"] == message["subject"] + with pytest.raises(ToolError): + await nc_mcp.call("get_mail_message", message_id=message["id"]) + + @pytest.mark.asyncio + async def test_move_keeps_flags(self, nc_mcp: McpTestHelper, archive_mailbox: int) -> None: + _, _, message = await _deliver_and_find(nc_mcp, "move-flags") + await nc_mcp.call("set_mail_message_flags", message_id=message["id"], flagged=True) + await nc_mcp.call("move_mail_message", message_id=message["id"], destination_mailbox_id=archive_mailbox) + await _sync_mailbox(nc_mcp, archive_mailbox) + moved = await _find_message(nc_mcp, archive_mailbox, message["subject"]) + assert moved is not None + assert "flagged" in moved.get("flags", []) + + @pytest.mark.asyncio + async def test_nonexistent_message_raises(self, nc_mcp: McpTestHelper, archive_mailbox: int) -> None: + with pytest.raises(ToolError, match="was not found or is not accessible"): + await nc_mcp.call("move_mail_message", message_id=999999999, destination_mailbox_id=archive_mailbox) + + @pytest.mark.asyncio + async def test_nonexistent_destination_raises(self, nc_mcp: McpTestHelper) -> None: + _, _, message = await _deliver_and_find(nc_mcp, "move-nowhere") + with pytest.raises(ToolError, match="does not exist"): + await nc_mcp.call("move_mail_message", message_id=message["id"], destination_mailbox_id=999999999) + + +class TestSetMailMessageFlags: + @pytest.mark.asyncio + async def test_set_and_clear_seen_and_flagged(self, nc_mcp: McpTestHelper) -> None: + _, inbox_id, message = await _deliver_and_find(nc_mcp, "flags") + assert "seen" not in message.get("flags", []) + result = json.loads( + await nc_mcp.call("set_mail_message_flags", message_id=message["id"], seen=True, flagged=True) + ) + assert result == {"message_id": message["id"], "flags": {"seen": True, "flagged": True}} + listed = await _find_message(nc_mcp, inbox_id, message["subject"]) + assert listed is not None + assert {"seen", "flagged"} <= set(listed.get("flags", [])) + + await nc_mcp.call("set_mail_message_flags", message_id=message["id"], seen=False, flagged=False) + listed = await _find_message(nc_mcp, inbox_id, message["subject"]) + assert listed is not None + assert not {"seen", "flagged"} & set(listed.get("flags", [])) + + @pytest.mark.asyncio + async def test_only_passed_flags_change(self, nc_mcp: McpTestHelper) -> None: + _, _, message = await _deliver_and_find(nc_mcp, "flags-partial") + await nc_mcp.call("set_mail_message_flags", message_id=message["id"], seen=True, answered=True) + await nc_mcp.call("set_mail_message_flags", message_id=message["id"], flagged=True) + full = json.loads(await nc_mcp.call("get_mail_message", message_id=message["id"])) + assert {"seen", "answered", "flagged"} <= set(full.get("flags", [])) + + @pytest.mark.asyncio + async def test_requires_at_least_one_flag(self, nc_mcp: McpTestHelper) -> None: + with pytest.raises(ToolError, match="at least one of seen, flagged or answered"): + await nc_mcp.call("set_mail_message_flags", message_id=1) + + @pytest.mark.asyncio + async def test_nonexistent_message_raises(self, nc_mcp: McpTestHelper) -> None: + with pytest.raises(ToolError, match="was not found or is not accessible"): + await nc_mcp.call("set_mail_message_flags", message_id=999999999, seen=True) + + +class TestMailTags: + @pytest.mark.asyncio + async def test_create_same_tag_twice_returns_same_label(self, nc_mcp: McpTestHelper) -> None: + account_id = await _get_account_id(nc_mcp) + name = f"{UNIQUE}-tag-{uuid.uuid4().hex[:8]}" + first = json.loads(await nc_mcp.call("create_mail_tag", display_name=name, color="#0082c9")) + try: + assert first["display_name"] == name + assert first["imap_label"].startswith("$") + assert first["color"] == "#0082c9" + second = json.loads(await nc_mcp.call("create_mail_tag", display_name=name, color="#ff0000")) + assert second["id"] == first["id"] + assert second["imap_label"] == first["imap_label"] + assert second["color"] == "#0082c9" + finally: + await _delete_tag(nc_mcp, account_id, first["id"]) + + @pytest.mark.asyncio + async def test_tag_and_untag_message(self, nc_mcp: McpTestHelper) -> None: + account_id, inbox_id, message = await _deliver_and_find(nc_mcp, "tags") + tag = json.loads( + await nc_mcp.call("create_mail_tag", display_name=f"{UNIQUE} tag {uuid.uuid4().hex[:8]}", color="#00aa00") + ) + try: + added = json.loads( + await nc_mcp.call("add_mail_message_tag", message_id=message["id"], imap_label=tag["imap_label"]) + ) + assert added["message_id"] == message["id"] + assert added["tag"]["imap_label"] == tag["imap_label"] + + listed = await _find_message(nc_mcp, inbox_id, message["subject"]) + assert listed is not None + assert _tag_ref(tag) in listed.get("tags", []) + full = json.loads(await nc_mcp.call("get_mail_message", message_id=message["id"])) + assert _tag_ref(tag) in full.get("tags", []) + + again = json.loads( + await nc_mcp.call("add_mail_message_tag", message_id=message["id"], imap_label=tag["imap_label"]) + ) + assert again["tag"]["id"] == tag["id"] + + removed = json.loads( + await nc_mcp.call("remove_mail_message_tag", message_id=message["id"], imap_label=tag["imap_label"]) + ) + assert removed["tag"]["imap_label"] == tag["imap_label"] + listed = await _find_message(nc_mcp, inbox_id, message["subject"]) + assert listed is not None + assert _tag_ref(tag) not in listed.get("tags", []) + finally: + await _delete_tag(nc_mcp, account_id, tag["id"]) + + @pytest.mark.asyncio + async def test_unknown_label_raises(self, nc_mcp: McpTestHelper) -> None: + _, _, message = await _deliver_and_find(nc_mcp, "tag-unknown") + with pytest.raises(ToolError, match="create_mail_tag"): + await nc_mcp.call("add_mail_message_tag", message_id=message["id"], imap_label="$mcp_test_no_such_tag") + + @pytest.mark.asyncio + async def test_invalid_color_rejected(self, nc_mcp: McpTestHelper) -> None: + with pytest.raises(ToolError, match="Invalid color"): + await nc_mcp.call("create_mail_tag", display_name=f"{UNIQUE}-bad-color", color="red") + + @pytest.mark.asyncio + async def test_empty_display_name_rejected(self, nc_mcp: McpTestHelper) -> None: + with pytest.raises(ToolError, match="must not be empty"): + await nc_mcp.call("create_mail_tag", display_name=" ", color="#0082c9") + + class TestMailPermissions: @pytest.mark.asyncio async def test_read_only_allows_list_accounts(self, nc_mcp_read_only: McpTestHelper) -> None: @@ -407,6 +630,23 @@ async def test_read_only_blocks_send(self, nc_mcp_read_only: McpTestHelper) -> N body="no", ) + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("tool", "args"), + [ + ("move_mail_message", {"message_id": 1, "destination_mailbox_id": 2}), + ("set_mail_message_flags", {"message_id": 1, "seen": True}), + ("create_mail_tag", {"display_name": "blocked", "color": "#000000"}), + ("add_mail_message_tag", {"message_id": 1, "imap_label": "$blocked"}), + ("remove_mail_message_tag", {"message_id": 1, "imap_label": "$blocked"}), + ], + ) + async def test_read_only_blocks_triage_tools( + self, nc_mcp_read_only: McpTestHelper, tool: str, args: dict[str, object] + ) -> None: + with pytest.raises(ToolError, match=r"requires 'write' permission"): + await nc_mcp_read_only.call(tool, **args) + @pytest.mark.asyncio async def test_write_allows_send(self, nc_mcp_write: McpTestHelper) -> None: result = await nc_mcp_write.call("list_mail_accounts") @@ -421,3 +661,26 @@ async def test_write_allows_send(self, nc_mcp_write: McpTestHelper) -> None: body="permission test", ) assert "sent" in result.lower() + + @pytest.mark.asyncio + async def test_write_allows_triage_tools( + self, nc_mcp: McpTestHelper, nc_mcp_write: McpTestHelper, archive_mailbox: int + ) -> None: + account_id, _, message = await _deliver_and_find(nc_mcp, "perm-triage") + tag = json.loads( + await nc_mcp_write.call( + "create_mail_tag", display_name=f"{UNIQUE}-perm-{uuid.uuid4().hex[:8]}", color="#aa00aa" + ) + ) + try: + flags = json.loads(await nc_mcp_write.call("set_mail_message_flags", message_id=message["id"], seen=True)) + assert flags["flags"] == {"seen": True} + label = tag["imap_label"] + await nc_mcp_write.call("add_mail_message_tag", message_id=message["id"], imap_label=label) + await nc_mcp_write.call("remove_mail_message_tag", message_id=message["id"], imap_label=label) + moved = await nc_mcp_write.call( + "move_mail_message", message_id=message["id"], destination_mailbox_id=archive_mailbox + ) + assert "moved" in moved + finally: + await _delete_tag(nc_mcp, account_id, tag["id"]) diff --git a/tests/test_client_app_request.py b/tests/test_client_app_request.py new file mode 100644 index 0000000..53753b0 --- /dev/null +++ b/tests/test_client_app_request.py @@ -0,0 +1,130 @@ +"""Tests for NextcloudClient.app_request_json and _raise_for_app_status (non-OCS app JSON routes).""" + +import json +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import niquests +import pytest + +from nc_mcp_server.client import NextcloudClient, NextcloudError, _raise_for_app_status +from nc_mcp_server.config import Config + + +def _response(status_code: int, body: object = None, raw: bytes | None = None) -> niquests.Response: + resp = niquests.Response() + resp.status_code = status_code + if raw is not None: + resp._content = raw + elif body is not None: + resp._content = json.dumps(body).encode("utf-8") + resp.headers["Content-Type"] = "application/json" + else: + resp._content = b"" + return resp + + +def _client_returning(*responses: niquests.Response) -> tuple[NextcloudClient, MagicMock]: + client = NextcloudClient(Config(nextcloud_url="http://nc.test", user="admin", password="secret")) + session = MagicMock() + session.request = AsyncMock(side_effect=list(responses)) + session.auth = ("admin", "secret") + client._session = session + return client, session + + +def _mail_fail(message: str) -> dict[str, Any]: + return {"status": "fail", "data": {"message": message, "type": "OCA\\Mail\\Exception\\ClientException"}} + + +class TestAppRequestJson: + @pytest.mark.asyncio + async def test_calls_index_php_route_with_ocs_header_and_json_body(self) -> None: + client, session = _client_returning(_response(200, {"id": 1})) + await client.app_request_json("POST", "mail/api/tags", json_data={"displayName": "x", "color": "#000000"}) + args, kwargs = session.request.call_args + assert args == ("POST", "http://nc.test/index.php/apps/mail/api/tags") + assert kwargs["headers"]["OCS-APIRequest"] == "true" + assert kwargs["json"] == {"displayName": "x", "color": "#000000"} + + @pytest.mark.asyncio + async def test_omits_json_and_params_when_not_given(self) -> None: + client, session = _client_returning(_response(200, {"id": 1})) + await client.app_request_json("GET", "mail/api/messages/5") + _, kwargs = session.request.call_args + assert "json" not in kwargs + assert "params" not in kwargs + + @pytest.mark.asyncio + async def test_forwards_params(self) -> None: + client, session = _client_returning(_response(200, [])) + await client.app_request_json("GET", "mail/api/messages", params={"mailboxId": "3"}) + _, kwargs = session.request.call_args + assert kwargs["params"] == {"mailboxId": "3"} + + @pytest.mark.asyncio + async def test_returns_decoded_json_without_ocs_envelope(self) -> None: + tag = {"id": 7, "displayName": "Needs Reply", "imapLabel": "$needs_reply"} + client, _ = _client_returning(_response(200, tag)) + assert await client.app_request_json("PUT", "mail/api/messages/5/tags/%24needs_reply") == tag + + @pytest.mark.asyncio + async def test_empty_body_returns_none(self) -> None: + client, _ = _client_returning(_response(200)) + assert await client.app_request_json("POST", "mail/api/messages/5/move", json_data={"destFolderId": 9}) is None + + @pytest.mark.asyncio + async def test_empty_json_list_is_returned_as_is(self) -> None: + client, _ = _client_returning(_response(200, [])) + assert await client.app_request_json("PUT", "mail/api/messages/5/flags", json_data={"flags": {}}) == [] + + @pytest.mark.asyncio + async def test_error_uses_app_message_and_context(self) -> None: + client, _ = _client_returning(_response(400, _mail_fail("Mailbox 99 does not exist"))) + with pytest.raises(NextcloudError, match=r"^POST apps/mail/api/messages/5/move: Mailbox 99 does not exist$"): + await client.app_request_json("POST", "mail/api/messages/5/move", json_data={"destFolderId": 99}) + + @pytest.mark.asyncio + async def test_retries_once_when_cached_session_expired(self, monkeypatch: pytest.MonkeyPatch) -> None: + client, session = _client_returning(_response(401), _response(200, {"ok": True})) + session.auth = None + monkeypatch.setattr(client, "_reset_session", AsyncMock()) + assert await client.app_request_json("GET", "mail/api/messages/5") == {"ok": True} + assert session.request.await_count == 2 + + +class TestRaiseForAppStatus: + def test_ok_response_does_not_raise(self) -> None: + _raise_for_app_status(_response(200, {"id": 1})) + + def test_fail_body_message(self) -> None: + with pytest.raises(NextcloudError, match="The maximum length for displayName is 128") as exc_info: + _raise_for_app_status(_response(400, _mail_fail("The maximum length for displayName is 128"))) + assert exc_info.value.status_code == 400 + + def test_error_body_top_level_message(self) -> None: + body: dict[str, Any] = {"status": "error", "message": "Could not load message", "data": {}, "code": 0} + with pytest.raises(NextcloudError, match="Could not load message") as exc_info: + _raise_for_app_status(_response(500, body)) + assert exc_info.value.status_code == 500 + + def test_empty_list_body_falls_back_to_status_message(self) -> None: + with pytest.raises(NextcloudError, match="Forbidden") as exc_info: + _raise_for_app_status(_response(403, [])) + assert exc_info.value.status_code == 403 + + def test_non_json_body_falls_back_to_status_message(self) -> None: + with pytest.raises(NextcloudError, match="Not found"): + _raise_for_app_status(_response(404, raw=b"404")) + + def test_no_body_falls_back_to_http_code(self) -> None: + with pytest.raises(NextcloudError, match="HTTP 502"): + _raise_for_app_status(_response(502)) + + def test_non_string_message_is_ignored(self) -> None: + with pytest.raises(NextcloudError, match="HTTP 400"): + _raise_for_app_status(_response(400, {"status": "fail", "data": {"message": ["not", "a", "string"]}})) + + def test_context_prefix(self) -> None: + with pytest.raises(NextcloudError, match=r"^PUT apps/mail/api/messages/1/flags: Forbidden"): + _raise_for_app_status(_response(403, []), "PUT apps/mail/api/messages/1/flags") diff --git a/tests/test_mail_tools.py b/tests/test_mail_tools.py new file mode 100644 index 0000000..4667f13 --- /dev/null +++ b/tests/test_mail_tools.py @@ -0,0 +1,197 @@ +"""Unit tests for the Mail triage tools: argument validation, request building and output formatting.""" + +import json +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest +from mcp.server.fastmcp import FastMCP +from mcp.server.fastmcp.exceptions import ToolError + +from nc_mcp_server.client import NextcloudError +from nc_mcp_server.permissions import PermissionLevel, set_permission_level +from nc_mcp_server.tools import mail + +TAG = {"id": 4, "userId": "admin", "displayName": "Needs Reply", "imapLabel": "$needs_reply", "color": "#0082c9"} + + +@pytest.fixture +def mcp_with_mock_client(monkeypatch: pytest.MonkeyPatch) -> tuple[FastMCP, MagicMock]: + set_permission_level(PermissionLevel.DESTRUCTIVE) + mock_client = MagicMock() + mock_client.app_request_json = AsyncMock() + mock_client.ocs_get = AsyncMock() + monkeypatch.setattr(mail, "get_client", lambda: mock_client) + mcp = FastMCP("test-mail") + mail.register(mcp) + return mcp, mock_client + + +async def _call(mcp: FastMCP, tool: str, **args: Any) -> str: + return await mcp._tool_manager.call_tool(tool, args) + + +class TestFormatTags: + def test_object_keyed_by_label(self) -> None: + assert mail._format_tags({"$needs_reply": TAG}) == [ + {"display_name": "Needs Reply", "imap_label": "$needs_reply"} + ] + + def test_empty_list_when_message_has_no_tags(self) -> None: + assert mail._format_tags([]) == [] + + def test_missing_or_unexpected_value(self) -> None: + assert mail._format_tags(None) == [] + assert mail._format_tags("oops") == [] + assert mail._format_tags(["not-a-tag"]) == [] + + +class TestMessageOutputIncludesTags: + @pytest.mark.asyncio + async def test_list_mail_messages_includes_tags(self, mcp_with_mock_client: tuple[FastMCP, MagicMock]) -> None: + mcp, client = mcp_with_mock_client + client.ocs_get.return_value = [ + {"databaseId": 11, "subject": "tagged", "flags": {"seen": True}, "tags": {"$needs_reply": TAG}}, + {"databaseId": 10, "subject": "untagged", "flags": {}, "tags": []}, + ] + data = json.loads(await _call(mcp, "list_mail_messages", mailbox_id=3))["data"] + assert data[0]["tags"] == [{"display_name": "Needs Reply", "imap_label": "$needs_reply"}] + assert "tags" not in data[1] + + @pytest.mark.asyncio + async def test_get_mail_message_reads_tags_from_app_route( + self, mcp_with_mock_client: tuple[FastMCP, MagicMock] + ) -> None: + mcp, client = mcp_with_mock_client + client.ocs_get.return_value = {"id": 11, "subject": "tagged", "body": "hi", "flags": {"seen": True}} + client.app_request_json.return_value = {"databaseId": 11, "tags": {"$needs_reply": TAG}} + result = json.loads(await _call(mcp, "get_mail_message", message_id=11)) + client.app_request_json.assert_awaited_once_with("GET", "mail/api/messages/11") + assert result["tags"] == [{"display_name": "Needs Reply", "imap_label": "$needs_reply"}] + assert result["flags"] == ["seen"] + + +class TestMoveMailMessage: + @pytest.mark.asyncio + async def test_sends_destination_and_warns_about_old_id( + self, mcp_with_mock_client: tuple[FastMCP, MagicMock] + ) -> None: + mcp, client = mcp_with_mock_client + client.app_request_json.return_value = [] + result = await _call(mcp, "move_mail_message", message_id=5, destination_mailbox_id=9) + client.app_request_json.assert_awaited_once_with( + "POST", "mail/api/messages/5/move", json_data={"destFolderId": 9} + ) + assert "no longer valid" in result + + @pytest.mark.asyncio + async def test_forbidden_becomes_not_found_message(self, mcp_with_mock_client: tuple[FastMCP, MagicMock]) -> None: + mcp, client = mcp_with_mock_client + client.app_request_json.side_effect = NextcloudError("POST apps/mail/api/messages/5/move: Forbidden.", 403) + with pytest.raises(ToolError, match=r"Message 5 or mailbox 9 was not found or is not accessible"): + await _call(mcp, "move_mail_message", message_id=5, destination_mailbox_id=9) + + @pytest.mark.asyncio + async def test_other_errors_pass_through(self, mcp_with_mock_client: tuple[FastMCP, MagicMock]) -> None: + mcp, client = mcp_with_mock_client + client.app_request_json.side_effect = NextcloudError("Mailbox 9 does not exist", 400) + with pytest.raises(ToolError, match="Mailbox 9 does not exist"): + await _call(mcp, "move_mail_message", message_id=5, destination_mailbox_id=9) + + +class TestSetMailMessageFlags: + @pytest.mark.asyncio + async def test_requires_at_least_one_flag(self, mcp_with_mock_client: tuple[FastMCP, MagicMock]) -> None: + mcp, client = mcp_with_mock_client + with pytest.raises(ToolError, match="at least one of seen, flagged or answered"): + await _call(mcp, "set_mail_message_flags", message_id=5) + client.app_request_json.assert_not_awaited() + + @pytest.mark.asyncio + async def test_sends_only_passed_flags(self, mcp_with_mock_client: tuple[FastMCP, MagicMock]) -> None: + mcp, client = mcp_with_mock_client + client.app_request_json.return_value = [] + result = json.loads(await _call(mcp, "set_mail_message_flags", message_id=5, seen=False, answered=True)) + client.app_request_json.assert_awaited_once_with( + "PUT", "mail/api/messages/5/flags", json_data={"flags": {"seen": False, "answered": True}} + ) + assert result == {"message_id": 5, "flags": {"seen": False, "answered": True}} + + @pytest.mark.asyncio + async def test_forbidden_becomes_not_found_message(self, mcp_with_mock_client: tuple[FastMCP, MagicMock]) -> None: + mcp, client = mcp_with_mock_client + client.app_request_json.side_effect = NextcloudError("Forbidden.", 403) + with pytest.raises(ToolError, match="Message 5 was not found or is not accessible"): + await _call(mcp, "set_mail_message_flags", message_id=5, flagged=True) + + +class TestCreateMailTag: + @pytest.mark.asyncio + async def test_creates_and_formats_tag(self, mcp_with_mock_client: tuple[FastMCP, MagicMock]) -> None: + mcp, client = mcp_with_mock_client + client.app_request_json.return_value = TAG + result = json.loads(await _call(mcp, "create_mail_tag", display_name=" Needs Reply ", color="#0082c9")) + client.app_request_json.assert_awaited_once_with( + "POST", "mail/api/tags", json_data={"displayName": "Needs Reply", "color": "#0082c9"} + ) + assert result == {"id": 4, "display_name": "Needs Reply", "imap_label": "$needs_reply", "color": "#0082c9"} + + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("display_name", "color", "error"), + [ + (" ", "#0082c9", "must not be empty"), + ("x" * 129, "#0082c9", "at most 128 characters"), + ("Needs Reply", "red", "Invalid color 'red'"), + ("Needs Reply", "#12345", "Invalid color"), + ], + ) + async def test_rejects_invalid_input( + self, mcp_with_mock_client: tuple[FastMCP, MagicMock], display_name: str, color: str, error: str + ) -> None: + mcp, client = mcp_with_mock_client + with pytest.raises(ToolError, match=error): + await _call(mcp, "create_mail_tag", display_name=display_name, color=color) + client.app_request_json.assert_not_awaited() + + @pytest.mark.asyncio + async def test_accepts_short_hex_color(self, mcp_with_mock_client: tuple[FastMCP, MagicMock]) -> None: + mcp, client = mcp_with_mock_client + client.app_request_json.return_value = {**TAG, "color": "#fff"} + await _call(mcp, "create_mail_tag", display_name="Needs Reply", color="#fff") + client.app_request_json.assert_awaited_once() + + +class TestMessageTagging: + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("tool", "method"), [("add_mail_message_tag", "PUT"), ("remove_mail_message_tag", "DELETE")] + ) + async def test_url_encodes_label( + self, mcp_with_mock_client: tuple[FastMCP, MagicMock], tool: str, method: str + ) -> None: + mcp, client = mcp_with_mock_client + client.app_request_json.return_value = TAG + result = json.loads(await _call(mcp, tool, message_id=5, imap_label="$needs reply/x")) + client.app_request_json.assert_awaited_once_with( + method, "mail/api/messages/5/tags/%24needs%20reply%2Fx", json_data=None + ) + assert result == { + "message_id": 5, + "tag": {"id": 4, "display_name": "Needs Reply", "imap_label": "$needs_reply", "color": "#0082c9"}, + } + + @pytest.mark.asyncio + @pytest.mark.parametrize("tool", ["add_mail_message_tag", "remove_mail_message_tag"]) + async def test_rejects_empty_label(self, mcp_with_mock_client: tuple[FastMCP, MagicMock], tool: str) -> None: + mcp, client = mcp_with_mock_client + with pytest.raises(ToolError, match="imap_label must not be empty"): + await _call(mcp, tool, message_id=5, imap_label="") + client.app_request_json.assert_not_awaited() + + @pytest.mark.asyncio + async def test_forbidden_points_to_create_mail_tag(self, mcp_with_mock_client: tuple[FastMCP, MagicMock]) -> None: + mcp, client = mcp_with_mock_client + client.app_request_json.side_effect = NextcloudError("Forbidden.", 403) + with pytest.raises(ToolError, match=r"no tag with IMAP label '\$unknown'.*create_mail_tag"): + await _call(mcp, "add_mail_message_tag", message_id=5, imap_label="$unknown") From 11ff522bbf9748c4616b2bb51c489c4c12e90a20 Mon Sep 17 00:00:00 2001 From: Oleksandr Piskun Date: Tue, 15 Sep 2026 17:54:51 +0000 Subject: [PATCH 2/2] Add the Mail triage tools to the expected tool list test_server.py pins the registered tool names and count, so the five new Mail tools failed it with 161 == 156. Signed-off-by: Oleksandr Piskun --- tests/integration/test_server.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/integration/test_server.py b/tests/integration/test_server.py index 27a78fd..694b10b 100644 --- a/tests/integration/test_server.py +++ b/tests/integration/test_server.py @@ -13,6 +13,7 @@ EXPECTED_TOOLS = [ "add_circle_member", "add_comment", + "add_mail_message_tag", "assign_tag", "clear_user_status", "close_poll", @@ -31,6 +32,7 @@ "create_event", "create_form", "create_form_share", + "create_mail_tag", "create_options", "create_poll", "create_question", @@ -129,8 +131,10 @@ "list_users", "list_versions", "move_file", + "move_mail_message", "remove_circle_member", "remove_file_reminder", + "remove_mail_message_tag", "reorder_options", "reorder_questions", "restore_collective", @@ -142,6 +146,7 @@ "send_mail", "send_message", "set_file_reminder", + "set_mail_message_flags", "set_user_status", "submit_form", "trash_collective",