Skip to content

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

Merged
rutgerhofste merged 2 commits into
mainfrom
claude/sharp-maxwell-cb0to7
Sep 16, 2026
Merged

rutgerhofste merged 2 commits into
mainfrom
claude/sharp-maxwell-cb0to7

Conversation

@rutgerhofste

@rutgerhofste rutgerhofste commented Sep 16, 2026 •

Copy link
Copy Markdown
Member

Closes the gap in Odoo task 928: Squirrel could write a draft and edit one, but not send it. The only way out was re-typing the draft's text into mail_send, which puts a second, subtly different message on the wire and leaves the reviewed one sitting in Drafts -- the duplicate the user finds afterwards.

The decision

mail_send_draft takes a uid and nothing else. A draft is already a complete RFC 5322 message, so rebuilding one 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 mail client wrote. What the user approved is what leaves.

How it is wired

  • imap.fetch_outgoing hands the parsed message (plus its attachment filenames, off the same fetch) to smtp.send_existing.
  • send_existing 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 (ours always do, another client's need not -- it is what the Sent copy is matched on).
  • It then joins send's own _deliver_prepared, so the Bcc rule (on the Sent copy, off what leaves) and the SMTP SIZE check stay 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 and inviting a retry that sends it twice.
  • The tool reads the capability as getattr(provider, "send_draft", None) and refuses with the alternative named, so a backend without it is told about rather than crashed into.

Tests

tests/test_send_draft.py (15 unit tests) pins the delivered bytes, the two re-stamped headers, the Bcc split, recipients coming from the draft's own headers, the never-fatal removal, and the tool's confirm gate + capability refusal. Two GreenMail e2e tests draft, send and then find the message in the inbox, in Sent, and gone from Drafts -- one of them with an attachment and a thread, proving nothing was rebuilt.

make test (252 passed) and make lint are green. make test-int was not run -- no Docker in this session, so the two new GreenMail tests are unverified against a real server.

Companion

pantalytics/squirrel-mcp-admin#78 adds the Graph half, so Outlook accounts get the same tool. It needs a SQUIRREL_MCP_REF bump to this commit once merged.

🤖 Generated with Claude Code

https://claude.ai/code/session_01K2DMxTXFL1aubHfjjEYCB6

Squirrel could write a draft (mail_draft) and edit one (mail_edit_draft) but
not send it. The only way out was re-typing the draft's text into mail_send,
which puts a second, subtly different message on the wire and leaves the
reviewed one sitting in Drafts -- the duplicate the user finds afterwards.

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 one 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 mail 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 stay 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 and inviting a retry that sends it twice.

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 rather than crashed into.

Refs Odoo task 928.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K2DMxTXFL1aubHfjjEYCB6
@rutgerhofste
rutgerhofste marked this pull request as ready for review September 16, 2026 07:52
@rutgerhofste
rutgerhofste merged commit 5c5a184 into main Sep 16, 2026
3 checks passed
@rutgerhofste
rutgerhofste deleted the claude/sharp-maxwell-cb0to7 branch September 16, 2026 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants