diff --git a/CLAUDE.md b/CLAUDE.md index aa74249..ae5b2ce 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -160,6 +160,32 @@ namespaces are reserved so they plug in later as sibling providers + tool mixins `tests/test_sent_copy.py` pins the discovery, the flags, the deduplication and the never-fatal rule; the GreenMail e2e sends for real and reads the copy back out of Sent. +- **Sending a draft sends the draft, not a copy of its fields.** Squirrel could + write a draft and edit one but not send it, so the only way out was re-typing + the text into `mail_send` -- which puts a *second* message on the wire and + leaves the reviewed one in Drafts. `mail_send_draft` takes a **uid and + nothing else**, and that is the decision: a draft is already a complete RFC + 5322 message, so rebuilding it from the arguments the tool layer models would + quietly drop everything they do not -- the `multipart/related` an embedded + image lives in, the `In-Reply-To` that makes it a reply, a header another + client wrote. What the user approved is what leaves. `imap.fetch_outgoing` + hands the parsed message (plus its attachment filenames, off the same fetch) + to `smtp.send_existing`, which re-stamps exactly two headers -- `Date`, + because a draft's is when it was *written* and would sort the mail above + what the recipient has already read, and a `Message-ID` when the draft has + none -- and then joins `send`'s own `_deliver_prepared`, so the Bcc rule and + the SIZE check are one path rather than two. `provider.send_draft` is the + third thing only that class can do, after `_reply_headers` and + `_file_sent_copy`: the message lives on the IMAP side and the wire is on the + SMTP side. Removing the draft is last and, like the Sent copy, **never fatal** + -- the mail is with the recipient by then, so a failure rides back as + `draft_removed=False` rather than reporting a delivered message as + undelivered. The tool reads the capability as + `getattr(provider, "send_draft", None)` and refuses with the alternative + named, so a backend that has not got it is told about instead of crashed + into. `tests/test_send_draft.py` pins the bytes, the two headers and the + never-fatal rule; the GreenMail e2e drafts, sends and then finds it in the + inbox, in Sent, and gone from Drafts. - **A meeting is not an appointment, and CalDAV only does the second.** `calendar_create_event` takes `attendees` and `online_meeting`, and both are gated on a capability the provider declares -- `supports_attendees` / diff --git a/README.md b/README.md index c1fd6f1..5fe8046 100644 --- a/README.md +++ b/README.md @@ -39,10 +39,17 @@ is configured. | `mail_get_attachment` | Download one attachment | read-only | | `mail_draft` / `mail_edit_draft` | Create / update a draft in Drafts, attachments included | writes to Drafts | | `mail_send` | Send a message, attachments included — **requires `confirm=true`** | ⚠️ outgoing | +| `mail_send_draft` | Send a draft that already exists, then take it out of Drafts — **requires `confirm=true`** | ⚠️ outgoing | | `mail_move` | Move messages between folders — **requires `confirm=true`** | ⚠️ mutating | | `mail_flag` | Flag / unflag messages — the star, `flagged=false` clears it | reversible | | `mail_mark_read` | Mark messages read / unread, `read=false` clears it | reversible | +Drafting and sending are the other two halves: `mail_draft` writes the message, +the user reads it, and `mail_send_draft` puts *that* message on the wire and +removes it from Drafts. It takes a uid and nothing else on purpose — a draft is +already a complete message, so re-typing its text into `mail_send` would send a +second, subtly different one and leave the reviewed draft behind. + Marking and finding are two halves of one thing: `mail_flag` sets the star and `mail_search(flagged_only=true)` gets those messages back; `mail_mark_read` sets the read state and `mail_search(unseen_only=true)` gets back what is left diff --git a/scripts/mcp_smoke.py b/scripts/mcp_smoke.py index f090af8..8e762b3 100644 --- a/scripts/mcp_smoke.py +++ b/scripts/mcp_smoke.py @@ -23,6 +23,7 @@ "mail_draft", "mail_edit_draft", "mail_send", + "mail_send_draft", "mail_move", "mail_flag", "mail_mark_read", diff --git a/squirrel_mcp/knowledge.py b/squirrel_mcp/knowledge.py index f6bb3d2..8ee71f1 100644 --- a/squirrel_mcp/knowledge.py +++ b/squirrel_mcp/knowledge.py @@ -42,6 +42,16 @@ body and the account it will be sent from, and get explicit approval BEFORE calling it with confirm=true. The result's `from_address` is the address the message actually went out from -- repeat it back to the user. +- mail_send_draft: send a draft that already exists, by uid, and take it out of + Drafts. DESTRUCTIVE/OUTGOING, same confirm=true rule -- read the draft with + mail_read and show the user what is in it first, since this tool has no + recipients or body of its own to show. This is the tool for "send it" after + the user has reviewed a draft: it puts the reviewed message itself on the + wire, attachments, embedded images and thread included. NEVER re-type a + draft's text into mail_send instead -- that sends a second, different message + and leaves the draft behind, which is exactly the duplicate the user sees in + their mailbox afterwards. `draft_removed: false` means it was sent but the + draft is still there: say so, offer to delete it, and never send it again. - ATTACHMENTS go out via the `attachments` argument on mail_send, mail_draft and mail_edit_draft. Two forms, and the choice matters: to send on a file that is already in the mailbox -- forwarding an invoice, passing on a contract -- use @@ -113,7 +123,8 @@ Good habits: - Resolve folder/addressbook/calendar ids with the matching list_* tool first. -- Prefer drafting (mail_draft) and letting the user review over sending directly. +- Prefer drafting (mail_draft) and letting the user review over sending directly, + then mail_send_draft to put that same reviewed message out. - ALWAYS show the user exactly what will change and get approval before calling any write tool (send/move/create/update/delete) with confirm=true. - uids are per-collection identifiers returned by the search/read tools; pass the diff --git a/squirrel_mcp/providers/protocol.py b/squirrel_mcp/providers/protocol.py index 4aedeca..3358bcd 100644 --- a/squirrel_mcp/providers/protocol.py +++ b/squirrel_mcp/providers/protocol.py @@ -333,6 +333,31 @@ def send( """ ... + def send_draft(self, folder: str, uid: str) -> dict: + """Send a draft that already exists, and take it out of ``folder``. + + Returns what ``send`` returns plus ``'subject'``, ``'attachments'`` + (filenames) and ``'draft_removed'``. + + **What goes out is the draft, not a copy of its fields.** The tool + layer deliberately hands over a uid and nothing else: a draft is + already a complete message, and anything rebuilt from the parts the + tools model would lose the rest -- the MIME tree an embedded image + needs, the threading that makes it a reply, a header another client + wrote. The user approved what they read; that is what must leave. + + The same never-fatal rule as the Sent copy applies to removing the + draft, and for the same reason: by then the message is with the + recipient, so a failure here is reported in ``draft_removed`` rather + than raised. Raising would say a delivered message was not delivered + and invite a retry that sends it twice. + + The tool layer reads this through ``getattr(provider, "send_draft", + None)`` and refuses with an alternative when a backend has not got it, + so an older backend is told about rather than crashed into. + """ + ... + def move(self, folder: str, uids: List[str], destination: str) -> int: """Move messages from ``folder`` to ``destination``. Returns count moved.""" ... diff --git a/squirrel_mcp/providers/soverin/imap.py b/squirrel_mcp/providers/soverin/imap.py index f19ac4d..cf951cb 100644 --- a/squirrel_mcp/providers/soverin/imap.py +++ b/squirrel_mcp/providers/soverin/imap.py @@ -12,6 +12,7 @@ import imaplib import re import ssl +from email.message import Message from typing import Callable, List, Optional, Tuple, TypeVar from imap_tools import ( @@ -368,6 +369,44 @@ def op(mb: BaseMailBox) -> str: return self._run(op) + # ---- sending a draft that already exists ------------------------------ # + def fetch_outgoing(self, folder: str, uid: str) -> Tuple[Message, List[str]]: + """A draft as the message it already is, plus its attachment filenames. + + Returns the parsed RFC 5322 object rather than anything this package + models, because the caller is about to *send* it: a draft is already a + complete message, and recomposing it from the fields ``fetch_message`` + exposes would drop everything they do not cover -- the MIME tree an + inline image needs, the threading headers a reply carries, a header + another client wrote. The filenames ride along from the same fetch so + the tool can name what went out without asking the server twice. + """ + + def op(mb: BaseMailBox) -> Tuple[Message, List[str]]: + self._select(mb, folder) + msgs = list(mb.fetch(uid_list=[uid], mark_seen=False, limit=1)) + if not msgs: + raise MailNotFoundError(f"Draft uid {uid} not found in {folder}") + names = [att.filename for att in msgs[0].attachments if att.filename] + return msgs[0].obj, names + + return self._run(op) + + def delete_message(self, folder: str, uid: str) -> None: + """Remove one message, the way ``update_draft`` removes the revision + it replaced -- the same STORE + EXPUNGE, for the same kind of message. + + Only ever pointed at a draft that has just been delivered: the copy the + recipient got is in Sent by then, so what is being removed is the + working copy, not the mail. + """ + + def op(mb: BaseMailBox) -> None: + self._select(mb, folder) + mb.delete([uid]) + + self._run(op) + # ---- the copy in Sent -------------------------------------------------- # def sent_folder(self) -> str: """The folder this account keeps sent mail in. diff --git a/squirrel_mcp/providers/soverin/provider.py b/squirrel_mcp/providers/soverin/provider.py index d2c0633..f72a6ed 100644 --- a/squirrel_mcp/providers/soverin/provider.py +++ b/squirrel_mcp/providers/soverin/provider.py @@ -214,6 +214,45 @@ def send( result["sent_folder"] = folder return result + def send_draft(self, folder: str, uid: str) -> dict: + """Send the draft at ``uid`` and take it out of ``folder``. + + The third thing only this class can do, after ``_reply_headers`` and + ``_file_sent_copy``, and for the same reason: the message lives on the + IMAP side and the wire is on the SMTP side. What goes out is the draft + itself, byte for byte, so a reviewed message is the message that is + sent -- see ``smtp.send_existing`` for the two headers that are not. + + Removing the draft is the last step and, like the Sent copy, is never + allowed to fail the send: the mail is with the recipient by then, so + raising would report a delivered message as undelivered and invite a + retry that sends it twice. ``draft_removed`` carries the outcome up + instead. + """ + msg, attachments = self._imap.fetch_outgoing(folder, uid) + subject = msg["Subject"] or "" + result = self._smtp.send_existing(msg) + raw = result.pop("raw", b"") + sent_at = result.pop("sent_at", None) + saved, sent_folder = self._file_sent_copy(raw, result.get("message_id"), sent_at) + result["saved_to_sent"] = saved + result["sent_folder"] = sent_folder + result["subject"] = subject + result["attachments"] = attachments + result["draft_removed"] = self._remove_draft(folder, uid) + return result + + def _remove_draft(self, folder: str, uid: str) -> bool: + try: + self._imap.delete_message(folder, uid) + except Exception as exc: # noqa: BLE001 - the mail is already gone + logger.warning( + "Draft uid %s in %s was sent but could not be removed: %s", + uid, folder, exc, + ) + return False + return True + def _file_sent_copy( self, raw: bytes, diff --git a/squirrel_mcp/providers/soverin/smtp.py b/squirrel_mcp/providers/soverin/smtp.py index 292cf99..45984aa 100644 --- a/squirrel_mcp/providers/soverin/smtp.py +++ b/squirrel_mcp/providers/soverin/smtp.py @@ -10,8 +10,9 @@ import datetime import smtplib import ssl -from email.message import EmailMessage -from typing import List, Optional +from email.message import Message +from email.utils import formatdate, getaddresses, make_msgid +from typing import List, Optional, Sequence from ...logging_config import get_logger from ..protocol import MailAuthError, MailProviderError, OutgoingAttachment @@ -39,6 +40,18 @@ def _timeout_for(size_bytes: int) -> int: return SMTP_TIMEOUT + size_bytes // MIN_UPLOAD_BYTES_PER_SEC +def _header_recipients(msg: Message) -> List[str]: + """Every address a composed message is addressed to, To + Cc + Bcc.""" + pairs = getaddresses( + msg.get_all("To", []) + msg.get_all("Cc", []) + msg.get_all("Bcc", []) + ) + seen: List[str] = [] + for _, addr in pairs: + if addr and addr not in seen: + seen.append(addr) + return seen + + class SoverinSmtpClient: """Thin SMTP sender. One connection per send (simple and robust).""" @@ -105,7 +118,45 @@ def send( attachments=attachments, body_html=body_html, ) - recipients = all_recipients(to, cc, bcc) + return self._deliver_prepared(msg, all_recipients(to, cc, bcc)) + + def send_existing(self, msg: Message) -> dict: + """Deliver a message somebody already composed -- a draft -- as it stands. + + The counterpart of ``send``, and the reason both end in + ``_deliver_prepared``: a draft is already a complete RFC 5322 message, + so rebuilding it from fields the tool layer re-read would be a second, + subtly different message -- a different Message-ID, a different MIME + tree, and whatever the composer put in that the tools do not model + quietly gone. What leaves is what was reviewed. + + Two headers are the exception, and both would be wrong left alone: + + * ``Date`` is re-stamped to now. A draft's Date is when it was + *written*, and a message that went out this afternoon carrying this + morning's timestamp sorts into the recipient's inbox at the wrong + place -- above mail they have already read. + * ``Message-ID`` is minted when the draft has none (ours always do; + another client's need not), because it is what the Sent copy is + matched on and what a reply to this message threads onto. + + Recipients come from the message's own headers -- the draft is the + only statement of who it is for. + """ + recipients = _header_recipients(msg) + if not recipients: + raise MailProviderError( + "This draft has no recipients: fill in 'to' with mail_edit_draft first." + ) + del msg["Date"] + msg["Date"] = formatdate(localtime=True) + if not msg["Message-ID"]: + msg["Message-ID"] = make_msgid() + return self._deliver_prepared(msg, recipients) + + def _deliver_prepared(self, msg: Message, recipients: Sequence[str]) -> dict: + """Put a finished message on the wire. The one delivery path.""" + recipients = list(recipients) if not recipients: raise MailProviderError("No recipients: 'to' is required") @@ -148,7 +199,7 @@ def send( "sent_at": datetime.datetime.now(datetime.timezone.utc), } - def _deliver(self, msg: EmailMessage, recipients: List[str], size_bytes: int) -> None: + def _deliver(self, msg: Message, recipients: List[str], size_bytes: int) -> None: timeout = _timeout_for(size_bytes) if self._security == "ssl": with smtplib.SMTP_SSL( diff --git a/squirrel_mcp/schemas.py b/squirrel_mcp/schemas.py index 5993a48..c132061 100644 --- a/squirrel_mcp/schemas.py +++ b/squirrel_mcp/schemas.py @@ -176,6 +176,15 @@ class SendResult(BaseModel): default=None, description="Folder the copy was filed in (pass it to mail_search to find it)", ) + draft_removed: Optional[bool] = Field( + default=None, + description=( + "Only from mail_send_draft: whether the draft was taken out of the " + "Drafts folder after it went out. False means the message WAS sent " + "and the draft is still sitting there -- say so and offer to delete " + "it; never send it a second time. Null from every other send." + ), + ) class MoveResult(BaseModel): diff --git a/squirrel_mcp/tools/mail/compose.py b/squirrel_mcp/tools/mail/compose.py index b72d26c..f0aebee 100644 --- a/squirrel_mcp/tools/mail/compose.py +++ b/squirrel_mcp/tools/mail/compose.py @@ -1,4 +1,4 @@ -"""mail_draft, mail_edit_draft, mail_send. +"""mail_draft, mail_edit_draft, mail_send, mail_send_draft. Replying: three of these take a ``reply_to_uid``. It does two separate jobs, and they live in different places on purpose. @@ -11,6 +11,14 @@ untouched, and each backend threads the way its transport threads -- headers for IMAP/SMTP, a conversation for an API backend. The tool layer neither knows nor cares which. + +``mail_send_draft`` is the same split read from the other end. It takes a uid +and *nothing else*: a draft is already a complete message, so re-reading its +fields here and handing them to ``send`` would put a second, subtly different +message on the wire -- a new Message-ID, a rebuilt MIME tree, and whatever the +draft carried that these arguments do not model (an embedded image's +``multipart/related``, the threading that makes it a reply) quietly gone. The +user approved what they read in the draft; that is what leaves. """ from __future__ import annotations @@ -235,6 +243,68 @@ async def mail_edit_draft( attachments=[a.filename for a in files] if files is not None else None, ) + @self.app.tool( + title="Send Draft", + annotations=ToolAnnotations( + readOnlyHint=False, + destructiveHint=True, + idempotentHint=False, + openWorldHint=True, + ), + ) + async def mail_send_draft( + uid: str, + folder: str = "Drafts", + confirm: bool = False, + account: Optional[str] = None, + ) -> SendResult: + """Send a draft that already exists. OUTGOING -- requires confirm=true. + + Use this for a message the user has been reviewing: it sends the + draft EXACTLY as it stands -- its recipients, its attachments, its + embedded images, its place in a thread -- and then removes it from + ``folder``. Do NOT re-send the same text through mail_send instead: + that composes a second message and leaves the draft behind. + + ``uid`` is the draft's uid, as returned by mail_draft / + mail_edit_draft or found with mail_search in the Drafts folder. + Pass the same ``account`` the draft lives in. + + Read the draft with mail_read first and show the user its + recipients, subject, body and attachments -- there are no arguments + here to show them instead -- then call again with confirm=true. + + THE COPY IN SENT: ``saved_to_sent`` says whether the message was + also filed in the account's Sent folder (``sent_folder`` names it). + ``draft_removed`` says whether the draft is gone. False for either + means the message DID go out -- say so and do NOT send again. + """ + provider, sub = await self._get_provider(account, writes=True) + send_draft = getattr(provider, "send_draft", None) + if send_draft is None: + # A backend that silently did nothing here would leave the user + # believing a reviewed message had been sent, so say what will + # work instead rather than pretending. + raise ValidationError( + "This account's mail backend cannot send an existing draft. " + "Send the message with mail_send and delete the draft, or " + "use an IMAP/SMTP account." + ) + require_confirm(confirm, "Sending a draft") + result = await run_blocking(provider, send_draft, folder, uid) + self._track_usage(sub, "mail_send_draft") + return SendResult( + status="Draft sent", + message_id=result.get("message_id"), + recipients=result.get("recipients", []), + from_address=getattr(provider, "email", None) or None, + subject=result.get("subject"), + attachments=result.get("attachments", []), + saved_to_sent=result.get("saved_to_sent"), + sent_folder=result.get("sent_folder"), + draft_removed=result.get("draft_removed"), + ) + @self.app.tool( title="Send Mail", annotations=ToolAnnotations( diff --git a/tests/conftest.py b/tests/conftest.py index 7205c8d..66e4ddf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,6 +10,7 @@ AttachmentInfo, AttachmentPayload, FolderInfo, + MailNotFoundError, MessageDetail, MessageSummary, ) @@ -42,6 +43,13 @@ def __init__(self): # Flipped off to stand in for a send whose copy could not be filed -- # the message still went out, which is the whole point of the field. self.sent_copy_ok = True + # What mail_send_draft sent, as (folder, uid), and the outcome of + # taking the draft out afterwards -- flipped off to stand in for a + # delivered message whose draft survived it. + self.sent_drafts: list = [] + self.draft_removal_ok = True + # Drafts the fake still holds, so a send-then-look-again is observable. + self.draft_store: dict = {"900": {"subject": "Reviewed", "to": ["bob@example.com"]}} self._email = "me@example.com" @property @@ -181,6 +189,23 @@ def send( "sent_folder": "Sent" if self.sent_copy_ok else None, } + def send_draft(self, folder, uid) -> dict: + if uid not in self.draft_store: + raise MailNotFoundError(f"Draft uid {uid} not found in {folder}") + draft = self.draft_store[uid] + self.sent_drafts.append((folder, uid)) + if self.draft_removal_ok: + del self.draft_store[uid] + return { + "message_id": "", + "recipients": list(draft["to"]), + "subject": draft["subject"], + "attachments": ["doc.pdf"], + "saved_to_sent": self.sent_copy_ok, + "sent_folder": "Sent" if self.sent_copy_ok else None, + "draft_removed": self.draft_removal_ok, + } + def move(self, folder, uids, destination) -> int: self.moved.append((folder, uids, destination)) return len(uids) diff --git a/tests/integration/test_greenmail_e2e.py b/tests/integration/test_greenmail_e2e.py index b78e206..f917841 100644 --- a/tests/integration/test_greenmail_e2e.py +++ b/tests/integration/test_greenmail_e2e.py @@ -579,3 +579,75 @@ def test_an_excluded_word_removes_the_message(provider): excluded, _ = p.search("INBOX", f"{marker} -unsubscribe", limit=50) assert _find_by_subject(excluded, subject) is None + + +def test_a_draft_is_delivered_and_leaves_drafts(provider): + """The whole round trip against a real server: draft, send that draft, and + it is in the recipient's inbox, in Sent, and gone from Drafts. + + Only a server proves the last part. The delete is a STORE + EXPUNGE, and a + draft that is still listed afterwards is exactly the duplicate this tool + exists to stop. + """ + p, _ = provider + subject = f"E2E send draft {uuid.uuid4().hex[:8]}" + + uid = p.save_draft([TEST_EMAIL], subject, "Reviewed, then sent.", folder="Drafts") + result = p.send_draft("Drafts", uid) + + assert TEST_EMAIL in result["recipients"] + assert result["subject"] == subject + assert result["draft_removed"] is True + assert result["saved_to_sent"] is True + + inbox, _ = p.search("INBOX", limit=50) + assert _find_by_subject(inbox, subject) is not None, "never arrived" + + sent, _t = p.search(result["sent_folder"], limit=50) + assert _find_by_subject(sent, subject) is not None, "no copy in Sent" + + drafts, _t = p.search("Drafts", limit=50) + assert _find_by_subject(drafts, subject) is None, "the draft is still in Drafts" + + +def test_a_sent_draft_keeps_its_attachment_and_its_thread(provider): + """What makes sending the draft itself the right call: the file and the + threading headers survive because nothing was rebuilt from re-read fields. + """ + p, cfg = provider + subject = f"E2E draft parts {uuid.uuid4().hex[:8]}" + parent_id = f"<{uuid.uuid4().hex}@external.test>" + _seed_message(cfg.smtp_host, cfg.smtp_port, subject, "Original.", message_id=parent_id) + + inbox, _ = p.search("INBOX", limit=50) + original = _find_by_subject(inbox, subject) + assert original is not None + + reply_subject = f"Re: {subject}" + uid = p.save_draft( + [TEST_EMAIL], + reply_subject, + "Answering, with the contract.", + folder="Drafts", + reply_to_uid=original.uid, + attachments=[ + OutgoingAttachment( + filename="contract.pdf", + content_type="application/pdf", + content=b"%PDF-1.4 contract bytes", + ) + ], + ) + result = p.send_draft("Drafts", uid) + assert result["attachments"] == ["contract.pdf"] + + inbox, _ = p.search("INBOX", limit=50) + arrived = _find_by_subject(inbox, reply_subject) + assert arrived is not None + + detail = p.fetch_message("INBOX", arrived.uid) + assert [a.filename for a in detail.attachments] == ["contract.pdf"] + assert p.fetch_attachment("INBOX", arrived.uid, 0).content == b"%PDF-1.4 contract bytes" + + raw, _names = p._imap.fetch_outgoing("INBOX", arrived.uid) + assert raw["In-Reply-To"] == parent_id diff --git a/tests/test_send_draft.py b/tests/test_send_draft.py new file mode 100644 index 0000000..cfaf919 --- /dev/null +++ b/tests/test_send_draft.py @@ -0,0 +1,253 @@ +"""Sending a draft that already exists. + +The gap this closes: Squirrel could write a draft and edit it, but not send +it. The only way out was to re-type the draft's text into ``mail_send``, which +puts a *second* message on the wire and leaves the reviewed one sitting in +Drafts -- the duplicate the user finds afterwards. + +The decision that makes it correct is that the draft is sent **as it stands**. +A draft is already a complete RFC 5322 message; rebuilding one from the fields +the tools model would drop everything they do not -- the ``multipart/related`` +an embedded image lives in, the ``In-Reply-To`` that makes it a reply, a header +another mail client wrote. So the tool takes a uid and nothing else, and the +bytes on the wire are the bytes that were reviewed. Two headers are the +exception and are tested here: ``Date``, which would otherwise say when the +draft was written, and a missing ``Message-ID``. + +The rest is the send path's existing contract read for a second caller: the +Bcc header stays on the Sent copy and comes off what leaves, the Sent copy is +never allowed to fail the send -- and neither is removing the draft, for +exactly the same reason. +""" + +from __future__ import annotations + +import email +import email.utils +from email.message import EmailMessage + +import pytest + +from squirrel_mcp.config import SquirrelConfig +from squirrel_mcp.providers.protocol import MailProviderError +from squirrel_mcp.providers.soverin.provider import SoverinMailProvider +from squirrel_mcp.providers.soverin.smtp import SoverinSmtpClient +from squirrel_mcp.server import create_fastmcp_app +from squirrel_mcp.tools import register_tools + + +@pytest.fixture +def app_with_tools(fake_provider): + app = create_fastmcp_app() + cfg = SquirrelConfig( + mail_email="me@example.com", + mail_password="pw", + imap_host="imap.x.eu", + smtp_host="smtp.x.eu", + ) + register_tools(app, fake_provider, cfg) + return app + + +def _draft(*, bcc: str = "", message_id: str = "") -> EmailMessage: + msg = EmailMessage() + msg["From"] = "me@example.net" + msg["To"] = "Anna , bob@example.com" + msg["Cc"] = "carol@example.com" + if bcc: + msg["Bcc"] = bcc + msg["Subject"] = "Reviewed and ready" + if message_id: + msg["Message-ID"] = message_id + msg["Date"] = email.utils.formatdate(0, localtime=False) # 1970 + msg.set_content("The body the user approved.") + return msg + + +class FakeSmtp(SoverinSmtpClient): + """Everything up to the socket, and nothing past it.""" + + def __init__(self): + super().__init__("smtp.example.net", 465, "me@example.net", "pw", "me@example.net") + self.delivered: list = [] + + def _deliver(self, msg, recipients, size_bytes): + self.delivered.append((msg.as_bytes(), list(recipients))) + + +# ---- SMTP: what actually leaves ------------------------------------------- # +def test_recipients_come_from_the_drafts_own_headers(): + """There are no ``to``/``cc`` arguments on this path -- the draft is the + only statement of who it is for, Bcc included.""" + smtp = FakeSmtp() + result = smtp.send_existing(_draft(bcc="dan@example.com")) + assert result["recipients"] == [ + "anna@example.com", + "bob@example.com", + "carol@example.com", + "dan@example.com", + ] + + +def test_the_delivered_bytes_are_the_draft(): + """The whole point: what was reviewed is what goes out. Rebuilding the + message from re-read fields would be a second, subtly different one.""" + smtp = FakeSmtp() + smtp.send_existing(_draft()) + sent = email.message_from_bytes(smtp.delivered[-1][0]) + assert sent["Subject"] == "Reviewed and ready" + assert sent.get_payload(decode=True) == b"The body the user approved.\n" + + +def test_the_message_id_survives_so_the_sent_copy_matches(): + smtp = FakeSmtp() + result = smtp.send_existing(_draft()) + assert result["message_id"] == "" + + +def test_a_draft_without_a_message_id_gets_one(): + """Ours always have one; another client's need not. It is what the Sent + copy is deduplicated on and what a reply to this message threads onto.""" + smtp = FakeSmtp() + result = smtp.send_existing(_draft(message_id="")) + assert result["message_id"] + assert email.message_from_bytes(smtp.delivered[-1][0])["Message-ID"] == result["message_id"] + + +def test_the_date_is_restamped_to_the_moment_it_was_sent(): + """A draft's Date is when it was *written*. Left alone, a message sent + this afternoon lands in the recipient's inbox above mail they have already + read.""" + smtp = FakeSmtp() + smtp.send_existing(_draft()) + sent = email.message_from_bytes(smtp.delivered[-1][0]) + assert email.utils.parsedate_to_datetime(sent["Date"]).year > 1970 + + +def test_bcc_stays_on_the_sent_copy_and_comes_off_what_leaves(): + """Your own Sent folder is the only record of whom you blind-copied; the + recipients must not be handed that list. Same rule as ``send``.""" + smtp = FakeSmtp() + result = smtp.send_existing(_draft(bcc="dan@example.com")) + assert b"dan@example.com" in result["raw"] # the copy for Sent + assert b"Bcc:" not in smtp.delivered[-1][0] + # ...and dan is still delivered to, over the envelope. + assert "dan@example.com" in smtp.delivered[-1][1] + + +def test_a_draft_with_no_recipients_is_refused_by_name(): + msg = EmailMessage() + msg["Subject"] = "Notes to self" + msg.set_content("...") + with pytest.raises(MailProviderError, match="no recipients"): + FakeSmtp().send_existing(msg) + + +# ---- provider: send, file the copy, then remove the draft ------------------ # +class FakeImap: + def __init__(self, msg=None, delete_raises=False): + self._msg = msg if msg is not None else _draft() + self.deleted: list = [] + self.appended: list = [] + self._delete_raises = delete_raises + + def fetch_outgoing(self, folder, uid): + return self._msg, ["contract.pdf"] + + def delete_message(self, folder, uid): + if self._delete_raises: + raise OSError("connection reset") + self.deleted.append((folder, uid)) + + def append_sent(self, raw, *, message_id=None, when=None): + self.appended.append(raw) + return "Sent", True + + +def _provider(imap, smtp=None) -> SoverinMailProvider: + p = SoverinMailProvider.__new__(SoverinMailProvider) + p._imap = imap + p._smtp = smtp or FakeSmtp() + p._drafts_folder = "Drafts" + p._email = "me@example.net" + return p + + +def test_the_draft_is_removed_after_it_goes_out(): + imap = FakeImap() + result = _provider(imap).send_draft("Drafts", "900") + assert imap.deleted == [("Drafts", "900")] + assert result["draft_removed"] is True + + +def test_the_copy_is_filed_in_sent_before_the_draft_is_dropped(): + """Order matters: the only surviving copy must exist somewhere else before + this one is deleted.""" + imap = FakeImap() + result = _provider(imap).send_draft("Drafts", "900") + assert imap.appended, "nothing was filed in Sent" + assert result["saved_to_sent"] is True and result["sent_folder"] == "Sent" + + +def test_a_failed_removal_does_not_fail_the_send(): + """The mail is with the recipient by now. Raising here would report a + delivered message as undelivered and invite a retry that sends it twice -- + the same rule the Sent copy follows.""" + imap = FakeImap(delete_raises=True) + result = _provider(imap).send_draft("Drafts", "900") + assert result["message_id"] == "" + assert result["draft_removed"] is False + + +def test_the_subject_and_attachments_ride_back_for_the_user(): + result = _provider(FakeImap()).send_draft("Drafts", "900") + assert result["subject"] == "Reviewed and ready" + assert result["attachments"] == ["contract.pdf"] + + +# ---- the tool ------------------------------------------------------------- # +def _payload(result) -> dict: + return result[1] if isinstance(result, tuple) else result + + +async def test_it_refuses_without_confirm_and_sends_nothing(app_with_tools, fake_provider): + with pytest.raises(Exception, match="confirm=true"): + await app_with_tools.call_tool("mail_send_draft", {"uid": "900"}) + assert fake_provider.sent_drafts == [] + assert "900" in fake_provider.draft_store + + +async def test_it_sends_the_draft_and_reports_what_happened(app_with_tools, fake_provider): + result = await app_with_tools.call_tool( + "mail_send_draft", {"uid": "900", "confirm": True} + ) + assert fake_provider.sent_drafts == [("Drafts", "900")] + assert "900" not in fake_provider.draft_store + payload = _payload(result) + assert payload["subject"] == "Reviewed" + assert payload["recipients"] == ["bob@example.com"] + assert payload["draft_removed"] is True + assert payload["sent_folder"] == "Sent" + + +async def test_a_draft_that_survived_its_own_send_is_reported_not_hidden( + app_with_tools, fake_provider +): + fake_provider.draft_removal_ok = False + result = await app_with_tools.call_tool( + "mail_send_draft", {"uid": "900", "confirm": True} + ) + payload = _payload(result) + assert payload["draft_removed"] is False + assert payload["message_id"] # it WAS sent -- never send it again + + +async def test_a_backend_without_the_capability_is_told_what_will_work( + app_with_tools, fake_provider, monkeypatch +): + """A backend that quietly did nothing here would leave the user believing + a reviewed message had been sent. Same refusal shape as attachments.""" + monkeypatch.delattr(type(fake_provider), "send_draft") + with pytest.raises(Exception, match="cannot send an existing draft"): + await app_with_tools.call_tool("mail_send_draft", {"uid": "900", "confirm": True}) + assert "900" in fake_provider.draft_store # untouched diff --git a/tests/test_tools.py b/tests/test_tools.py index 18f8120..a38e93b 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -19,6 +19,7 @@ "mail_draft", "mail_edit_draft", "mail_send", + "mail_send_draft", "mail_move", "mail_flag", "mail_mark_read",