diff --git a/.changeset/fileref-content-id.md b/.changeset/fileref-content-id.md new file mode 100644 index 00000000..db15f165 --- /dev/null +++ b/.changeset/fileref-content-id.md @@ -0,0 +1,16 @@ +--- +"@plotday/twister": minor +--- + +Added: `contentId` on `ActionType.fileRef` actions. + +Marks a file reference as an inline body image rather than an attachment. Set +it to the part's Content-ID (without angle brackets) when the note's content +still carries a matching `cid:` reference, and Plot renders the image where the +sender placed it, at its own size, instead of appending an attachment chip. + +Mail connectors should use this to distinguish three cases: an ordinary +attachment (no `contentId`), an inline image the retained content still +references (`contentId` set), and an inline image whose only reference lived in +quoted history that was trimmed away — a signature logo, typically — which +should be dropped rather than surfaced as an attachment nobody can place. diff --git a/connectors/apple/src/mail/transform.test.ts b/connectors/apple/src/mail/transform.test.ts index 660776f3..c755966f 100644 --- a/connectors/apple/src/mail/transform.test.ts +++ b/connectors/apple/src/mail/transform.test.ts @@ -445,6 +445,55 @@ describe("transformMessages attachments", () => { expect(note.actions).toBeUndefined(); }); + /** One inline image part, as BODYSTRUCTURE reports a `multipart/related` image. */ + const inlinePart = (contentId: string) => ({ + partNumber: "2", + fileName: "image001.jpg", + mimeType: "image/jpeg", + size: 823, + encoding: "base64", + contentId, + inline: true, + }); + + it("tags an inline image the body references with its Content-ID", () => { + const m = msg({ + uid: 15, + bodyHtml: `

Here it is:

`, + attachments: [inlinePart("ii_abc123")], + }); + const note = transform([m])[0].notes![0] as unknown as { + actions?: Array; + }; + expect(note.actions).toHaveLength(1); + expect(note.actions![0].contentId).toBe("ii_abc123"); + }); + + it("drops an inline image the body never references", () => { + const m = msg({ + uid: 16, + bodyHtml: `

No image here.

`, + attachments: [inlinePart("ii_orphan")], + }); + const note = transform([m])[0].notes![0] as unknown as { actions?: ActionLike[] }; + expect(note.actions).toBeUndefined(); + }); + + it("keeps an inline image when only a plain-text body survived", () => { + // No HTML to match a `cid:` against, so the part can't be shown to be + // orphaned — keep it rather than silently discard an attachment. + const m = msg({ + uid: 17, + bodyText: "Can we meet?", + attachments: [inlinePart("ii_abc123")], + }); + const note = transform([m])[0].notes![0] as unknown as { + actions?: Array; + }; + expect(note.actions).toHaveLength(1); + expect(note.actions![0].contentId ?? null).toBeNull(); + }); + it("FIX 6: omits an inline calendar part whose fileName is the synthesized 'attachment' placeholder", () => { const m = msg({ uid: 15, diff --git a/connectors/apple/src/mail/transform.ts b/connectors/apple/src/mail/transform.ts index 24840867..505083a8 100644 --- a/connectors/apple/src/mail/transform.ts +++ b/connectors/apple/src/mail/transform.ts @@ -1,5 +1,6 @@ import type { ImapAddress, ImapMessage } from "@plotday/twister/tools/imap"; import { ActionType, type Action, type NewContact, type NewLinkWithNotes } from "@plotday/twister"; +import { referencedContentIds } from "@plotday/twister/signals"; import { parse } from "../product-channel"; import { appleMailSignals } from "./apple-facets"; @@ -163,18 +164,40 @@ function compareCopies(a: MailMessage, b: MailMessage): number { * chip on emails that don't even bundle (bare invites). A genuinely named * calendar attachment (e.g. a forwarded `invite.ics`) still appears * normally — only the synthesized-name case is suppressed. + * + * Inline images are classified against the message's HTML body: one the body + * references is tagged with its Content-ID so Plot renders it in place, while + * one nothing references is dropped — it has nowhere to render, and would + * otherwise put an attachment chip on the message for an image the reader never + * saw. A message with no HTML body carries no `cid:` references to match + * against, so there inline parts stay attachments rather than being silently + * discarded. */ function attachmentActions(m: MailMessage): Action[] | undefined { if (!m.attachments || m.attachments.length === 0) return undefined; - const actions = m.attachments - .filter((a) => !(isCalendarAttachment(a.mimeType) && a.fileName === "attachment")) - .map((a) => ({ + const body = bodyOf(m); + const referenced = + body && body.contentType === "html" + ? referencedContentIds(body.content) + : null; + const actions: Action[] = []; + for (const a of m.attachments) { + if (isCalendarAttachment(a.mimeType) && a.fileName === "attachment") continue; + const inlineId = a.inline && a.contentId ? a.contentId : null; + const embedded = + inlineId !== null && + referenced !== null && + referenced.has(inlineId.toLowerCase()); + if (inlineId !== null && referenced !== null && !embedded) continue; + actions.push({ type: ActionType.fileRef as ActionType.fileRef, ref: buildAttachmentRef(m.mailbox, m.uid, a.partNumber), fileName: a.fileName, fileSize: a.size, mimeType: a.mimeType, - })); + ...(embedded ? { contentId: inlineId } : {}), + }); + } return actions.length > 0 ? actions : undefined; } diff --git a/connectors/google/src/mail/gmail-api.test.ts b/connectors/google/src/mail/gmail-api.test.ts index e478209b..d38ca173 100644 --- a/connectors/google/src/mail/gmail-api.test.ts +++ b/connectors/google/src/mail/gmail-api.test.ts @@ -1307,3 +1307,140 @@ describe("extractCalendarReplies", () => { }); }); +describe("transformGmailThread inline images", () => { + /** + * A `multipart/related` message: the HTML body plus one image part carried + * the way a mail client attaches an inline image — `Content-Disposition: + * inline` with a `Content-ID` the HTML points at via `src="cid:…"`. + */ + function relatedThread(html: string, contentId = "ii_abc123"): GmailThread { + return thread({ + from: "Robin ", + to: "me@example.com", + subject: "Re: Query", + payload: part("multipart/related", { + parts: [ + part("multipart/alternative", { + parts: [part("text/html", { data: html })], + }), + { + mimeType: "image/jpeg", + filename: "image001.jpg", + headers: [ + { name: "Content-Type", value: 'image/jpeg; name="image001.jpg"' }, + { + name: "Content-Disposition", + value: 'inline; filename="image001.jpg"', + }, + { name: "Content-ID", value: `<${contentId}>` }, + { name: "X-Attachment-Id", value: contentId }, + ], + body: { size: 823, attachmentId: "att-inline-1" }, + }, + ], + }), + }); + } + + function actionsOf(link: ReturnType) { + return (link.notes![0].actions ?? []) as Array<{ + fileName: string; + contentId?: string | null; + }>; + } + + it("drops an inline image whose only reference was in the trimmed quote", () => { + // The signature logo lives inside the quoted history, which + // `stripQuotedReply` cuts away — nothing in the retained body points at it. + const link = transformGmailThread( + relatedThread( + `
Brilliant! Thank you.
` + + `
` + + `

Robin Fisher

` + + `
` + ) + ); + + expect(actionsOf(link)).toEqual([]); + }); + + it("keeps an inline image the retained body references, tagged with its Content-ID", () => { + const link = transformGmailThread( + relatedThread( + `
Here is the chart:
` + + `
` + ) + ); + + expect(actionsOf(link)).toEqual([ + expect.objectContaining({ + fileName: "image001.jpg", + contentId: "ii_abc123", + }), + ]); + }); + + it("leaves an ordinary attachment untagged", () => { + const link = transformGmailThread( + thread({ + from: "Robin ", + to: "me@example.com", + subject: "Report", + payload: part("multipart/mixed", { + parts: [ + part("text/html", { data: "
See attached.
" }), + { + mimeType: "application/pdf", + filename: "report.pdf", + headers: [ + { + name: "Content-Disposition", + value: 'attachment; filename="report.pdf"', + }, + ], + body: { size: 40201, attachmentId: "att-pdf-1" }, + }, + ], + }), + }) + ); + + const actions = actionsOf(link); + expect(actions).toHaveLength(1); + expect(actions[0].fileName).toBe("report.pdf"); + expect(actions[0].contentId ?? null).toBeNull(); + }); + + it("keeps an inline image when only a plain-text body survived", () => { + // Nothing to match a `cid:` against, so the part can't be shown to be + // orphaned — keep it rather than silently discard an attachment. + const link = transformGmailThread( + thread({ + from: "Robin ", + to: "me@example.com", + subject: "Re: Query", + payload: part("multipart/related", { + parts: [ + part("text/plain", { data: "Brilliant! Thank you." }), + { + mimeType: "image/jpeg", + filename: "image001.jpg", + headers: [ + { + name: "Content-Disposition", + value: 'inline; filename="image001.jpg"', + }, + { name: "Content-ID", value: "" }, + ], + body: { size: 823, attachmentId: "att-inline-1" }, + }, + ], + }), + }) + ); + + expect(actionsOf(link)).toHaveLength(1); + }); +}); + diff --git a/connectors/google/src/mail/gmail-api.ts b/connectors/google/src/mail/gmail-api.ts index a1349797..b8b18cf5 100644 --- a/connectors/google/src/mail/gmail-api.ts +++ b/connectors/google/src/mail/gmail-api.ts @@ -9,7 +9,11 @@ import type { } from "@plotday/twister/plot"; import { markdownToPlainText } from "@plotday/twister/utils/markdown"; import { markdownToHtml } from "@plotday/twister/utils/markdown-html"; -import { isNoReplySender } from "@plotday/twister/signals"; +import { + isNoReplySender, + normalizeContentId, + referencedContentIds, +} from "@plotday/twister/signals"; import { icsProp, parseIcsReply } from "@plotday/rsvp-fold"; @@ -1161,9 +1165,23 @@ function isForwardedMessage(content: string): boolean { return true; } +/** A part's header value by name (case-insensitive), or null. */ +function partHeader(part: GmailMessagePart, name: string): string | null { + const lower = name.toLowerCase(); + return ( + (part.headers ?? []).find((h) => h.name.toLowerCase() === lower)?.value ?? + null + ); +} + /** * Recursively collects attachment parts from a Gmail message payload. * An attachment part has a non-empty filename and a body.attachmentId. + * + * `contentId` and `inline` describe how the part was carried, so callers can + * tell a real attachment from an image the HTML body embeds via `cid:`. A + * `multipart/related` image routinely omits `Content-Disposition` altogether, + * so a bare `Content-ID` counts as inline too. */ export function collectAttachments( part: GmailMessagePart | undefined @@ -1172,19 +1190,29 @@ export function collectAttachments( fileName: string; fileSize: number | null; mimeType: string; + contentId: string | null; + inline: boolean; }> { if (!part) return []; - const here = - part.filename && part.body?.attachmentId - ? [ - { - partId: part.body.attachmentId, - fileName: part.filename, - fileSize: part.body?.size ?? null, - mimeType: part.mimeType ?? "application/octet-stream", - }, - ] - : []; + let here: ReturnType = []; + if (part.filename && part.body?.attachmentId) { + // RFC 2392: the `cid:` URL names the Content-ID without its angle + // brackets, so strip them here rather than at every comparison site. + const contentId = normalizeContentId(partHeader(part, "Content-ID")); + const disposition = partHeader(part, "Content-Disposition"); + here = [ + { + partId: part.body.attachmentId, + fileName: part.filename, + fileSize: part.body?.size ?? null, + mimeType: part.mimeType ?? "application/octet-stream", + contentId, + inline: disposition + ? /^\s*inline\b/i.test(disposition) + : contentId !== null, + }, + ]; + } const children = (part.parts ?? []).flatMap(collectAttachments); return [...here, ...children]; } @@ -1310,14 +1338,32 @@ export function transformGmailThread(thread: GmailThread): NewLinkWithNotes { const content = body || message.snippet; - // Build fileRef actions for each attachment part - const actions: Action[] = attachmentParts.map((a) => ({ - type: ActionType.fileRef as ActionType.fileRef, - ref: `${message.id}:${a.partId}`, - fileName: a.fileName, - fileSize: a.fileSize, - mimeType: a.mimeType, - })); + // Build fileRef actions for each attachment part. Inline images are + // classified against the body we KEPT: one the retained content still + // references is tagged with its Content-ID so Plot renders it in place, + // while one whose only reference was trimmed away with the quoted history + // is dropped. Only meaningful for an HTML body we actually kept — a + // plain-text body carries no `cid:` references to match against, so there + // the parts stay attachments rather than being silently discarded. + const referenced = + contentType === "html" && body.trim() ? referencedContentIds(body) : null; + const actions: Action[] = []; + for (const a of attachmentParts) { + const inlineId = a.inline && a.contentId ? a.contentId : null; + const embedded = + inlineId !== null && + referenced !== null && + referenced.has(inlineId.toLowerCase()); + if (inlineId !== null && referenced !== null && !embedded) continue; + actions.push({ + type: ActionType.fileRef as ActionType.fileRef, + ref: `${message.id}:${a.partId}`, + fileName: a.fileName, + fileSize: a.fileSize, + mimeType: a.mimeType, + ...(embedded ? { contentId: inlineId } : {}), + }); + } // Note author (sender) and per-message recipients for visibility. // source is populated so the DM recipient picker can resolve Gmail contacts. diff --git a/connectors/outlook/src/mail/graph-mail-api.test.ts b/connectors/outlook/src/mail/graph-mail-api.test.ts index 93e2fe3a..e39260d7 100644 --- a/connectors/outlook/src/mail/graph-mail-api.test.ts +++ b/connectors/outlook/src/mail/graph-mail-api.test.ts @@ -102,7 +102,7 @@ describe("transformOutlookConversation", () => { expect(team?.name).toBeUndefined(); }); - it("emits fileRef actions for non-inline file attachments only", () => { + it("emits fileRef actions for file attachments, dropping unreferenced inline images", () => { const atts = new Map([ [ "id-1", @@ -144,6 +144,54 @@ describe("transformOutlookConversation", () => { expect(actions).toHaveLength(1); expect(actions[0].ref).toBe("id-1:a1"); }); + + /** One inline image attachment carrying `contentId`, as Graph reports it. */ + const inlineImage = (contentId: string): GraphAttachmentMeta => ({ + id: "a-inline", + name: "image001.jpg", + contentType: "image/jpeg", + size: 823, + isInline: true, + odataType: "#microsoft.graph.fileAttachment", + contentId, + }); + + function inlineActions(html: string, contentId = "ii_abc123") { + const link = transformOutlookConversation({ + ...base, + attachmentsByMessageId: new Map([["id-1", [inlineImage(contentId)]]]), + messages: [ + msg({ + hasAttachments: true, + body: { contentType: "html", content: html }, + }), + ], + }); + return ( + link.notes![0] as { + actions: Array<{ ref: string; contentId?: string | null }> | null; + } + ).actions; + } + + it("keeps an inline image the retained body references, tagged with its Content-ID", () => { + const actions = inlineActions( + `

Here it is:

` + ); + expect(actions).toHaveLength(1); + expect(actions![0].ref).toBe("id-1:a-inline"); + expect(actions![0].contentId).toBe("ii_abc123"); + }); + + it("drops an inline image whose only reference was in the trimmed quote", () => { + // `stripQuotedReply` cuts at the reply header block, so the signature logo + // below it is gone from the body the reader sees. + const actions = inlineActions( + `

Thanks!

` + + `

Regards, Robin

` + ); + expect(actions ?? []).toHaveLength(0); + }); }); describe("conversation state helpers", () => { diff --git a/connectors/outlook/src/mail/graph-mail-api.ts b/connectors/outlook/src/mail/graph-mail-api.ts index 1799f623..6a137f72 100644 --- a/connectors/outlook/src/mail/graph-mail-api.ts +++ b/connectors/outlook/src/mail/graph-mail-api.ts @@ -6,7 +6,11 @@ import type { NewContact, NewLinkWithNotes, } from "@plotday/twister/plot"; -import { isNoReplySender } from "@plotday/twister/signals"; +import { + isNoReplySender, + normalizeContentId, + referencedContentIds, +} from "@plotday/twister/signals"; import type { RsvpReply } from "@plotday/rsvp-fold"; import { stripQuotedReply } from "./email-parsing"; @@ -63,6 +67,12 @@ export type GraphAttachmentMeta = { isInline: boolean; /** "#microsoft.graph.fileAttachment" | itemAttachment | referenceAttachment */ odataType: string; + /** + * Content-ID of an inline part, without angle brackets — what the body's + * `cid:` reference names. Null for ordinary attachments, and for inline + * parts Graph reports without one. + */ + contentId?: string | null; }; /** Well-known folder name → folder id map (only the ones we care about). */ @@ -402,7 +412,7 @@ export class GraphMailApi { const data = (await this.call( "GET", `${GRAPH}/me/messages/${encodeURIComponent(messageId)}/attachments`, - { $select: "id,name,contentType,size,isInline" } + { $select: "id,name,contentType,size,isInline,contentId" } )) as { value?: Array> } | null; return ((data?.value ?? []) as Array>).map((a) => ({ id: a.id as string, @@ -411,6 +421,9 @@ export class GraphMailApi { size: (a.size as number | undefined) ?? null, isInline: (a.isInline as boolean | undefined) ?? false, odataType: (a["@odata.type"] as string | undefined) ?? "", + contentId: normalizeContentId( + (a.contentId as string | undefined) ?? null + ), })); } @@ -823,20 +836,35 @@ export function transformOutlookConversation(opts: { const body = stripQuotedReply(message.body?.content ?? "", contentType); const content = body || message.bodyPreview || ""; - const actions: Action[] = ( - opts.attachmentsByMessageId.get(message.id) ?? [] - ) - .filter( - (a) => - !a.isInline && a.odataType === "#microsoft.graph.fileAttachment" - ) - .map((a) => ({ + // Inline parts are classified against the body we KEPT: one the retained + // content still references is tagged with its Content-ID so Plot renders it + // in place, while one whose only reference was trimmed away with the quoted + // history is dropped. Only meaningful for an HTML body we actually kept — + // a plain-text body carries no `cid:` references to match against, so there + // inline parts stay attachments rather than being silently discarded. + const referenced = + contentType === "html" && body.trim() ? referencedContentIds(body) : null; + const actions: Action[] = []; + for (const a of opts.attachmentsByMessageId.get(message.id) ?? []) { + if (a.odataType !== "#microsoft.graph.fileAttachment") continue; + const inlineId = a.isInline && a.contentId ? a.contentId : null; + const embedded = + inlineId !== null && + referenced !== null && + referenced.has(inlineId.toLowerCase()); + // An inline part Graph reports without a contentId can never be matched + // to the body, so it is treated as an orphan whenever we have a body to + // check — the same conclusion the old blanket `!isInline` filter reached. + if (a.isInline && referenced !== null && !embedded) continue; + actions.push({ type: ActionType.fileRef as ActionType.fileRef, ref: `${message.id}:${a.id}`, fileName: a.name, fileSize: a.size, mimeType: a.contentType ?? "application/octet-stream", - })); + ...(embedded ? { contentId: inlineId } : {}), + }); + } const senderActor: NewActor = { email: fromAddress, diff --git a/twister/src/plot.ts b/twister/src/plot.ts index f2853e88..5c8f0105 100644 --- a/twister/src/plot.ts +++ b/twister/src/plot.ts @@ -321,6 +321,21 @@ export type Action = imageWidth?: number | null; /** Intrinsic height of the image in pixels (only for image files) */ imageHeight?: number | null; + /** + * Content-ID of an inline body image, without the angle brackets — the + * value a `cid:` reference in the note's content resolves against. + * + * Set this ONLY when the note's content still references the part (an + * `` that survived quote-trimming). Plot then renders + * the image in the body where the sender placed it, at its own size, + * instead of appending an attachment chip. + * + * Leave it unset for ordinary attachments. An inline part the retained + * content no longer references — a signature logo left behind when a + * quoted reply was trimmed — should be dropped entirely rather than + * emitted with a `contentId`, since nothing in the body points at it. + */ + contentId?: string | null; } | { /** Thread reference action for navigating to a related thread */ diff --git a/twister/src/signals.ts b/twister/src/signals.ts index b5ba2ba7..dc65fbbe 100644 --- a/twister/src/signals.ts +++ b/twister/src/signals.ts @@ -102,3 +102,53 @@ function localPart(address: string | null): string { export function isNoReplySender(address: string | null): boolean { return NOREPLY_LOCALPART.test(localPart(address)); } + +/** + * Content-IDs an HTML mail body still points at, lowercased for comparison. + * + * Mail connectors use this to classify a message's inline image parts against + * the body they actually kept, after quoted history has been trimmed away. + * Three outcomes follow: + * + * - Referenced → set the fileRef action's `contentId` so Plot renders the + * image in the body where the sender placed it. + * - Not referenced → the part is an orphan. Its only reference lived in the + * quoted history that was trimmed, which is what happens to a sender's + * signature logo on every reply in a chain. Drop it: there is nowhere to + * render it, and surfacing it as an attachment puts a chip on every message + * for an image the reader never saw. + * - No HTML body to test against (a plain-text message) → skip the check + * entirely rather than treat every inline part as an orphan. + * + * Values are compared lowercased because clients are inconsistent about the + * case they echo a Content-ID back in. + */ +export function referencedContentIds(html: string): Set { + const ids = new Set(); + // The src of `` in any quoting style, stopping at whatever + // terminates the URL. + const re = /\bcid:([^"'\s>)]+)/gi; + let match: RegExpExecArray | null; + while ((match = re.exec(html)) !== null) { + const raw = match[1]; + let decoded = raw; + try { + // RFC 2392 percent-encodes characters that are special in a URL. + decoded = decodeURIComponent(raw); + } catch { + // Malformed escape — compare the raw value instead. + } + ids.add(decoded.toLowerCase()); + } + return ids; +} + +/** + * Strips the angle brackets RFC 2822 wraps a `Content-ID` header in, leaving + * the bare value a `cid:` URL names (RFC 2392). Returns null for a missing or + * empty header. + */ +export function normalizeContentId(header: string | null): string | null { + if (!header) return null; + return header.trim().replace(/^<|>$/g, "") || null; +} diff --git a/twister/src/tools/imap.ts b/twister/src/tools/imap.ts index 337b3a60..e48a18d3 100644 --- a/twister/src/tools/imap.ts +++ b/twister/src/tools/imap.ts @@ -143,8 +143,22 @@ export type ImapMessage = { * `partNumber` is the IMAP part number (e.g. "2" or "2.1") used to fetch that * part's content separately, and `encoding` is the part's own * Content-Transfer-Encoding. + * + * `contentId` (angle brackets stripped) and `inline` describe how the part + * was carried, so a body image the HTML embeds via `cid:` can be told apart + * from a genuine attachment. A part sent without an explicit + * `Content-Disposition` but with a `Content-ID` counts as inline — that is + * how `multipart/related` images usually arrive. */ - attachments?: { partNumber: string; fileName: string; mimeType: string; size: number; encoding: string }[]; + attachments?: { + partNumber: string; + fileName: string; + mimeType: string; + size: number; + encoding: string; + contentId?: string | null; + inline?: boolean; + }[]; }; /** Options for fetchMessages(). */