diff --git a/lib/mail_mcp/imap_client.rb b/lib/mail_mcp/imap_client.rb index aa7a29b..9b12886 100644 --- a/lib/mail_mcp/imap_client.rb +++ b/lib/mail_mcp/imap_client.rb @@ -190,12 +190,15 @@ def format_addresses(addrs) def extract_attachments(mail) mail.attachments.map do |att| + # Mail#decoded does not memoize — it base64-decodes into a new String on + # every call, so decode once and reuse for both the upload and the size. + content = att.decoded url = AttachmentStore.upload( - content: att.decoded, + content: content, filename: att.filename || "attachment", content_type: att.content_type ) - { filename: att.filename, content_type: att.content_type, size: att.decoded.bytesize, url: url } + { filename: att.filename, content_type: att.content_type, size: content.bytesize, url: url } end end end diff --git a/spec/mail_mcp/imap_client_spec.rb b/spec/mail_mcp/imap_client_spec.rb index 0b962ec..d2b3985 100644 --- a/spec/mail_mcp/imap_client_spec.rb +++ b/spec/mail_mcp/imap_client_spec.rb @@ -50,6 +50,63 @@ end end + describe "#get_message" do + let(:attachment_body) { "PDF-CONTENT" * 10 } + let(:raw_message) do + mail = Mail.new + mail.from = "alice@example.com" + mail.to = "bob@example.com" + mail.subject = "Invoice" + mail.text_part = Mail::Part.new { body "see attached" } + mail.add_file(filename: "invoice.pdf", content: attachment_body) + mail.to_s + end + + before do + fetch_data = instance_double( + Net::IMAP::FetchData, + attr: { "RFC822" => raw_message, "FLAGS" => [:Seen] } + ) + allow(imap).to receive(:uid_fetch).and_return([fetch_data]) + allow(MailMCP::AttachmentStore).to receive(:upload).and_return("https://s3.example.com/invoice.pdf") + end + + it "returns the message with its attachment metadata" do + result = described_class.new(imap).get_message(folder: "INBOX", uid: 42) + + expect(result[:subject]).to eq("Invoice") + expect(result[:attachments]).to contain_exactly( + { + filename: "invoice.pdf", + content_type: a_string_including("application/pdf"), + size: attachment_body.bytesize, + url: "https://s3.example.com/invoice.pdf" + } + ) + end + + # Mail#decoded re-decodes on every call, so decoding twice doubled the peak + # memory of the largest allocation in the request path. + it "decodes each attachment only once" do + decode_count = 0 + allow_any_instance_of(Mail::Part).to receive(:decoded).and_wrap_original do |original| # rubocop:disable RSpec/AnyInstance + # Count only the attachment: format_message legitimately decodes + # text_part/html_part too, and those are Mail::Part instances as well. + decode_count += 1 if original.receiver.attachment? + original.call + end + + described_class.new(imap).get_message(folder: "INBOX", uid: 42) + + expect(decode_count).to eq(1) + end + + it "returns nil when the uid is not found" do + allow(imap).to receive(:uid_fetch).and_return([]) + expect(described_class.new(imap).get_message(folder: "INBOX", uid: 99)).to be_nil + end + end + describe "#search_messages" do it "passes raw query string to IMAP SEARCH" do allow(imap).to receive(:search).with(["UNSEEN"]).and_return([1, 2, 3])