Add mail_send_draft: send the draft that was reviewed, not a copy of it

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` /

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

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",

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

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."""
         ...

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.

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,

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(

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):