From f7426d09c68b4b44949352d49728fd2ea56f61cc Mon Sep 17 00:00:00 2001 From: Kris Braun Date: Mon, 3 Aug 2026 16:15:45 -0400 Subject: [PATCH 01/10] refactor(apple): read ICS properties through the shared library --- connectors/apple/package.json | 1 + .../apple/src/mail/calendar-bundle.test.ts | 49 +++++++++++++++++++ connectors/apple/src/mail/calendar-bundle.ts | 19 +------ pnpm-lock.yaml | 3 ++ 4 files changed, 55 insertions(+), 17 deletions(-) diff --git a/connectors/apple/package.json b/connectors/apple/package.json index 6de53508..b6db4c51 100644 --- a/connectors/apple/package.json +++ b/connectors/apple/package.json @@ -29,6 +29,7 @@ "test": "vitest run" }, "dependencies": { + "@plotday/rsvp-fold": "workspace:^", "@plotday/twister": "workspace:^" }, "devDependencies": { diff --git a/connectors/apple/src/mail/calendar-bundle.test.ts b/connectors/apple/src/mail/calendar-bundle.test.ts index 72057f75..61125403 100644 --- a/connectors/apple/src/mail/calendar-bundle.test.ts +++ b/connectors/apple/src/mail/calendar-bundle.test.ts @@ -109,3 +109,52 @@ describe("classifyICS — the full classification matrix", () => { expect(classifyICS(folded)).toEqual({ uid: "evt-8-part1-part2", kind: "cancel" }); }); }); + +describe("classifyICS — property reading after the shared-icsProp swap", () => { + it("reads a folded UID line (RFC 5545 continuation) the same as before", () => { + // A 75-octet line wrapped with CRLF + single space. The UID must come + // back joined, not truncated at the fold. + const ics = [ + "BEGIN:VCALENDAR", + "METHOD:CANCEL", + "BEGIN:VEVENT", + "UID:this-is-a-deliberately-long-identifier-that-the-generator-wrapped", + " -across-two-lines@example.test", + "SEQUENCE:1", + "END:VEVENT", + "END:VCALENDAR", + ].join("\r\n"); + + expect(classifyICS(ics)).toEqual({ + uid: "this-is-a-deliberately-long-identifier-that-the-generator-wrapped-across-two-lines@example.test", + kind: "cancel", + }); + }); + + it("ignores parameters on the property it reads", () => { + const ics = [ + "BEGIN:VCALENDAR", + "METHOD:REQUEST", + "BEGIN:VEVENT", + "UID;X-VENDOR-QUIRK=1:evt-params@example.test", + "SEQUENCE:3", + "END:VEVENT", + "END:VCALENDAR", + ].join("\r\n"); + + expect(classifyICS(ics)).toEqual({ uid: "evt-params@example.test", kind: "update" }); + }); + + it("still skips a METHOD:REPLY (folding is not this function's job)", () => { + const ics = [ + "BEGIN:VCALENDAR", + "METHOD:REPLY", + "BEGIN:VEVENT", + "UID:evt-reply@example.test", + "END:VEVENT", + "END:VCALENDAR", + ].join("\r\n"); + + expect(classifyICS(ics)).toBeNull(); + }); +}); diff --git a/connectors/apple/src/mail/calendar-bundle.ts b/connectors/apple/src/mail/calendar-bundle.ts index 23bd1a1e..335dcafb 100644 --- a/connectors/apple/src/mail/calendar-bundle.ts +++ b/connectors/apple/src/mail/calendar-bundle.ts @@ -15,6 +15,8 @@ * and hands the decoded text to `classifyICS`). */ +import { icsProp } from "@plotday/rsvp-fold"; + /** Raw classification of one ICS blob, before the mail sync pass resolves * whether the calendar product has already synced an event for that UID. */ export type ClassifiedICS = { uid: string; kind: "cancel" | "update" }; @@ -39,23 +41,6 @@ export function isCalendarAttachment(mimeType: string): boolean { return CALENDAR_MIME_TYPES.has(mimeType.toLowerCase()); } -/** - * Unfold RFC 5545 continuation lines (CRLF/LF + leading space/tab is a - * continuation of the previous line's value) and read a property's value. - * Unscoped — matches the property anywhere in the ICS text, which is - * correct for `METHOD` (a VCALENDAR-level property that sits outside - * `BEGIN:VEVENT`/`END:VEVENT`; the existing `parseICSEvents`/`parseVEvent` - * in `../calendar/ics-parser` parses only VEVENT-scoped properties and has - * no `method` field at all) as well as for `UID`/`SEQUENCE` (VEVENT-scoped, - * but a calendar invite email carries exactly one VEVENT). - */ -function icsProp(ics: string, name: string): string | null { - const unfolded = ics.replace(/\r?\n[ \t]/g, ""); - const re = new RegExp(`^${name}(?:;[^:\\r\\n]*)?:(.*)$`, "im"); - const m = unfolded.match(re); - return m ? m[1].trim() : null; -} - /** * Classify one ICS (VCALENDAR) text's relationship to its event, per the * product-approved rule (see module doc): diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 107e4995..461e3c85 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -39,6 +39,9 @@ importers: connectors/apple: dependencies: + '@plotday/rsvp-fold': + specifier: workspace:^ + version: link:../../libs/rsvp-fold '@plotday/twister': specifier: workspace:^ version: link:../../twister From 2efcd5ce52e30e7b818424a5dbfb554e75da88ed Mon Sep 17 00:00:00 2001 From: Kris Braun Date: Mon, 3 Aug 2026 16:29:32 -0400 Subject: [PATCH 02/10] fix(apple): keep examining new calendar messages in a settled thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A mail thread's root is its first References entry, and calendar systems thread response notifications onto the original invitation's Message-ID. The bundling cache is keyed on that root and short-circuited the entire root on a hit, so once the invitation had been ingested and its root recorded, no later message on that thread was ever looked at again. Split the gate. The cached bundling classification is still served unchanged on every pass — it must never flip, including on passes where the calendar-bearing message has aged out of the rescan window and this pass carries no calendar part at all. What no longer short-circuits is the per-message scan: a settled root now still fetches calendar parts it has not read before, tracked per message in ThreadMeta.seenIcs so a part already examined is not re-fetched on every poll. detectCalendarBundles now returns { bundles, foldedNoteKeys }; the second set is empty for now and exists so callers are written against the final shape. noteKeyOf is exported from transform.ts as the shared per-message identity both sets are keyed under. --- connectors/apple/src/mail/sync.test.ts | 160 ++++++++++++++++++++++--- connectors/apple/src/mail/sync.ts | 113 +++++++++++++---- connectors/apple/src/mail/transform.ts | 9 +- 3 files changed, 243 insertions(+), 39 deletions(-) diff --git a/connectors/apple/src/mail/sync.test.ts b/connectors/apple/src/mail/sync.test.ts index a08c87b6..2452626a 100644 --- a/connectors/apple/src/mail/sync.test.ts +++ b/connectors/apple/src/mail/sync.test.ts @@ -216,6 +216,44 @@ function daysAgo(days: number): Date { return new Date(Date.now() - days * DAY_MS); } +/** A message carrying an inline text/calendar part at partNumber "2". */ +function calendarMessage(opts: { + uid: number; + messageId: string; + root: string; + date?: Date; +}): ImapMessage { + return { + uid: opts.uid, + messageId: opts.messageId, + references: [opts.root], + subject: "Accepted: Weekly sync", + from: [{ address: "guest@example.test", name: "Sam Guest" }], + to: [{ address: "owner@example.test", name: "Owner" }], + date: opts.date ?? daysAgo(1), + flags: ["\\Seen"], + bodyText: "Sam Guest has accepted this invitation.", + attachments: [ + { partNumber: "2", fileName: "attachment", mimeType: "text/calendar", size: 400, encoding: "7bit" }, + ], + } as unknown as ImapMessage; +} + +/** A message with no calendar part at all. */ +function plainMessage(opts: { uid: number; root: string }): ImapMessage { + return { + uid: opts.uid, + messageId: ``, + references: [opts.root], + subject: "Re: Weekly sync", + from: [{ address: "guest@example.test", name: "Sam Guest" }], + to: [{ address: "owner@example.test", name: "Owner" }], + date: daysAgo(1), + flags: ["\\Seen"], + bodyText: "See you there.", + } as unknown as ImapMessage; +} + /** The plan history floor most fixtures run under — inside the 30-day window, * so `floor` and `recentSince` coincide and these tests exercise the merge * rather than the window. The window itself is exercised by the tests that @@ -1467,7 +1505,7 @@ describe("detectCalendarBundles", () => { const meta = metaFor(["invite@example.com"]); const changed = new Set(); - const bundles = await detectCalendarBundles( + const { bundles } = await detectCalendarBundles( host, "session-1", [{ ...m, mailbox: "INBOX" }], @@ -1500,7 +1538,7 @@ describe("detectCalendarBundles", () => { knownEventUids: ["evt-known"], }); - const bundles = await detectCalendarBundles( + const { bundles } = await detectCalendarBundles( host, "session-1", [{ ...m, mailbox: "INBOX" }], @@ -1525,7 +1563,7 @@ describe("detectCalendarBundles", () => { }, }); - const bundles = await detectCalendarBundles( + const { bundles } = await detectCalendarBundles( host, "session-1", [{ ...m, mailbox: "INBOX" }], @@ -1551,7 +1589,7 @@ describe("detectCalendarBundles", () => { }, }); - const bundles = await detectCalendarBundles( + const { bundles } = await detectCalendarBundles( host, "session-1", [{ ...m, mailbox: "INBOX" }], @@ -1572,7 +1610,7 @@ describe("detectCalendarBundles", () => { }, }); - const bundles = await detectCalendarBundles( + const { bundles } = await detectCalendarBundles( host, "session-1", [{ ...m, mailbox: "INBOX" }], @@ -1606,7 +1644,7 @@ describe("detectCalendarBundles", () => { }, }); - const bundles = await detectCalendarBundles( + const { bundles } = await detectCalendarBundles( host, "session-1", [ @@ -1628,7 +1666,7 @@ describe("detectCalendarBundles", () => { const m = msg({ uid: 56, messageId: "" }); const { host, fetchAttachmentCalls } = bundleHost({}); - const bundles = await detectCalendarBundles( + const { bundles } = await detectCalendarBundles( host, "session-1", [{ ...m, mailbox: "INBOX" }], @@ -1650,7 +1688,7 @@ describe("detectCalendarBundles", () => { }); const { host, fetchAttachmentCalls } = bundleHost({}); - const bundles = await detectCalendarBundles( + const { bundles } = await detectCalendarBundles( host, "session-1", [{ ...m, mailbox: "INBOX" }], @@ -1665,7 +1703,7 @@ describe("detectCalendarBundles", () => { it("returns an empty map for an empty message list (no I/O)", async () => { const { host, fetchAttachmentCalls } = bundleHost({}); - const bundles = await detectCalendarBundles(host, "session-1", [], new Map(), new Set()); + const { bundles } = await detectCalendarBundles(host, "session-1", [], new Map(), new Set()); expect(fetchAttachmentCalls).toHaveLength(0); expect(bundles.size).toBe(0); @@ -1685,7 +1723,7 @@ describe("detectCalendarBundles", () => { }, }); - const bundles = await detectCalendarBundles( + const { bundles } = await detectCalendarBundles( host, "session-1", [{ ...m, mailbox: SENT_BOX }], @@ -1714,7 +1752,7 @@ describe("detectCalendarBundles", () => { const meta = metaFor(["cached@example.com"]); const merged: MailMessage[] = [{ ...m, mailbox: "INBOX" }]; - const first = await detectCalendarBundles(host, "session-1", merged, meta, new Set()); + const { bundles: first } = await detectCalendarBundles(host, "session-1", merged, meta, new Set()); expect(first.get("cached@example.com")).toEqual({ uid: "evt-cached", kind: "cancel", @@ -1722,7 +1760,7 @@ describe("detectCalendarBundles", () => { }); expect(fetchAttachmentCalls).toHaveLength(1); - const second = await detectCalendarBundles(host, "session-1", merged, meta, new Set()); + const { bundles: second } = await detectCalendarBundles(host, "session-1", merged, meta, new Set()); expect(second.get("cached@example.com")).toEqual({ uid: "evt-cached", kind: "cancel", @@ -1753,7 +1791,7 @@ describe("detectCalendarBundles", () => { }); const meta = metaFor(["root-aged@example.com"]); - const first = await detectCalendarBundles( + const { bundles: first } = await detectCalendarBundles( host, "session-1", [ @@ -1773,7 +1811,7 @@ describe("detectCalendarBundles", () => { // Pass 2: only the in-window reply. Without the recorded decision the root // would silently un-bundle, flipping its primary `source` and creating a // duplicate link row. - const second = await detectCalendarBundles( + const { bundles: second } = await detectCalendarBundles( host, "session-1", [{ ...followUp, mailbox: "INBOX" }], @@ -1800,17 +1838,103 @@ describe("detectCalendarBundles", () => { const meta = metaFor(["bare-cached@example.com"]); const merged: MailMessage[] = [{ ...m, mailbox: "INBOX" }]; - const first = await detectCalendarBundles(host, "session-1", merged, meta, new Set()); + const { bundles: first } = await detectCalendarBundles(host, "session-1", merged, meta, new Set()); expect(first.has("bare-cached@example.com")).toBe(false); expect(fetchAttachmentCalls).toHaveLength(1); // "Evaluated, doesn't bundle" must stay distinguishable from "never // evaluated", hence the wrapping object. expect(meta.get("bare-cached@example.com")!.bundle).toEqual({ classified: null }); - const second = await detectCalendarBundles(host, "session-1", merged, meta, new Set()); + const { bundles: second } = await detectCalendarBundles(host, "session-1", merged, meta, new Set()); expect(second.has("bare-cached@example.com")).toBe(false); expect(fetchAttachmentCalls).toHaveLength(1); // reused the recorded decision }); + + it("still examines a NEW calendar message in a root whose bundle decision is already cached", async () => { + // The invite was ingested on an earlier pass and cached as "no bundle" + // (REQUEST/SEQUENCE 0). A reply then threads onto that same root. Before + // this gate split, the cache hit short-circuited the whole root and the + // reply's ICS was never fetched at all. + const replyIcs = ics({ method: "REPLY", uid: "evt-cached" }); + const reply = calendarMessage({ + uid: 51, + messageId: "", + root: "", + }); + const { host, fetchAttachmentCalls } = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [reply])], + attachments: { [buildAttachmentRef("INBOX", 51, "2")]: icsBytes(replyIcs) }, + }); + const fixtureMessages: MailMessage[] = [{ ...reply, mailbox: "INBOX" }]; + + const meta = new Map([ + ["invite@example.test", { channelId: "INBOX", bundle: { classified: null } }], + ]); + const changed = new Set(); + + await detectCalendarBundles(host, "session-1", fixtureMessages, meta, changed); + + expect(fetchAttachmentCalls).toHaveLength(1); + expect(meta.get("invite@example.test")!.seenIcs).toEqual(["reply-1@example.test"]); + expect(changed.has("invite@example.test")).toBe(true); + }); + + it("does not re-fetch a calendar part it has already examined", async () => { + const reply = calendarMessage({ + uid: 51, + messageId: "", + root: "", + }); + const { host, fetchAttachmentCalls } = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [reply])], + attachments: {}, // a fetch would throw on lookup miss — that IS the assertion + }); + const fixtureMessages: MailMessage[] = [{ ...reply, mailbox: "INBOX" }]; + + const meta = new Map([ + [ + "invite@example.test", + { channelId: "INBOX", bundle: { classified: null }, seenIcs: ["reply-1@example.test"] }, + ], + ]); + + await detectCalendarBundles(host, "session-1", fixtureMessages, meta, new Set()); + + expect(fetchAttachmentCalls).toHaveLength(0); + }); + + it("keeps serving the cached bundle decision — a classification must never flip", async () => { + const plain = plainMessage({ uid: 60, root: "" }); + const { host } = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [plain])], + knownEventUids: ["evt-cached"], + }); + const fixtureMessages: MailMessage[] = [{ ...plain, mailbox: "INBOX" }]; + + const meta = new Map([ + [ + "invite@example.test", + { channelId: "INBOX", bundle: { classified: { uid: "evt-cached", kind: "cancel" } } }, + ], + ]); + + const { bundles } = await detectCalendarBundles( + host, + "session-1", + fixtureMessages, + meta, + new Set() + ); + + expect(bundles.get("invite@example.test")).toEqual({ + uid: "evt-cached", + kind: "cancel", + eventKnown: true, + }); + }); }); describe("mailSync — calendar thread bundling end-to-end", () => { @@ -1839,10 +1963,12 @@ describe("mailSync — calendar thread bundling end-to-end", () => { expect("title" in link).toBe(false); expect(stored.get("cancel-email:evt-e2e")).toBeTruthy(); // The decision is persisted on the root's single metadata document, - // alongside its home channel. + // alongside its home channel and the note keys whose calendar part has + // been read (what keeps a later pass from re-fetching the same ICS). expect(stored.get("thread:cancel-e2e@example.com")).toEqual({ channelId: "mail:INBOX", bundle: { classified: { uid: "evt-e2e", kind: "cancel" } }, + seenIcs: ["cancel-e2e@example.com"], }); }); diff --git a/connectors/apple/src/mail/sync.ts b/connectors/apple/src/mail/sync.ts index 4a7d955a..51b59905 100644 --- a/connectors/apple/src/mail/sync.ts +++ b/connectors/apple/src/mail/sync.ts @@ -12,6 +12,7 @@ import type { MailboxCursor, MailHost, MailSyncState } from "./mail-host"; import { mailSource, messageKey, + noteKeyOf, rootMessageId, transformMessages, type MailMessage, @@ -74,8 +75,27 @@ export type ThreadMeta = { * distinguishable (see `detectCalendarBundles`'s CACHING doc). */ bundle?: { classified: ClassifiedICS | null }; + /** + * Note keys (see `noteKeyOf`) whose calendar part has already been fetched + * and examined. Distinct from `bundle`, which is a per-ROOT decision that + * must never flip: this is per-MESSAGE, and exists so a root whose bundling + * question is already settled still looks at messages that arrived since. + * + * Required, not an optimisation. `alreadyFolded` stops a duplicate note but + * not the IMAP fetch, and inside the 30-day rescan window an RSVP would + * otherwise be re-fetched on every pass — roughly 2,900 times, at up to two + * round-trips each, against a ~1,000-request execution budget. + * + * Capped at SEEN_ICS_MAX, oldest dropped. Growth is bounded in practice by + * how many responses one meeting draws, but this document is rewritten every + * pass and an unbounded array is not worth the tail risk. + */ + seenIcs?: string[]; }; +/** Cap on `ThreadMeta.seenIcs`; see its doc. */ +const SEEN_ICS_MAX = 200; + function threadMetaKey(rootId: string): string { return `thread:${rootId}`; } @@ -250,6 +270,20 @@ export async function reconcileTodoFlags( * on file. Persisting the decision once means the classification can never * flip after the fact. * + * That cache short-circuits the per-ROOT decision ONLY. Per-MESSAGE + * examination continues on every pass: a root whose bundling question is + * already settled still fetches calendar parts it has not read before + * (tracked in `ThreadMeta.seenIcs`). This matters because a root is the first + * `References` entry and calendar systems thread response notifications onto + * the invitation's Message-ID — so those later messages arrive on a root that + * was settled by the pass which ingested the invitation, and gating the whole + * root on the cache would mean never looking at them. + * + * Returns the per-root `bundles` map plus `foldedNoteKeys`: note keys of + * messages this function consumed itself, so the caller can leave them out of + * the thread's notes. Nothing is consumed yet — the set is always empty — but + * callers are written against the final shape. + * * The caller owns the store I/O: `meta` arrives pre-loaded (one read per root * for the pass, shared with home-channel resolution) and is MUTATED in place * with any new decision, with the root added to `changed` so the caller @@ -267,8 +301,9 @@ export async function detectCalendarBundles( messages: MailMessage[], meta: Map, changed: Set -): Promise> { +): Promise<{ bundles: Map; foldedNoteKeys: Set }> { const bundles = new Map(); + const foldedNoteKeys = new Set(); let knownUids: Set | null = null; const resolveEventKnown = async (uid: string): Promise => { if (knownUids === null) knownUids = await host.knownEventUids(); @@ -276,11 +311,22 @@ export async function detectCalendarBundles( }; for (const [root, msgs] of groupByRoot(messages).entries()) { - // Reuse an earlier pass's decision before doing anything else — see the - // CACHING doc above for why this must not be gated behind "does THIS - // pass have a calendar part" (the ICS-bearing message may have aged out - // of the window while the thread itself is still active). - const persisted = meta.get(root)?.bundle; + const entry = meta.get(root); + + // The cached BUNDLE decision is served unchanged — see the CACHING doc: + // a classification that flips changes `sources`' sorted-minimum primary + // source and makes `upsert_link` create a second link row. It is consulted + // before anything else, so a root whose ICS-bearing message has aged out + // of the window keeps its decision even when this pass's messages carry + // no calendar part at all. + // + // What is deliberately NOT skipped is the per-message scan below. A root + // is the first `References` entry, and calendar systems thread response + // notifications onto the invite's Message-ID — so the root carrying an + // RSVP is usually the invite's root, already cached as "no bundle" from + // the pass that ingested the invite. Returning early here (as this code + // used to) means an RSVP is never looked at. + const persisted = entry?.bundle; if (persisted) { if (persisted.classified) { bundles.set(root, { @@ -288,25 +334,48 @@ export async function detectCalendarBundles( eventKnown: await resolveEventKnown(persisted.classified.uid), }); } - continue; } - // Not yet classified. Cheap in-memory check (no I/O) — only - // calendar-bearing threads ever touch IMAP below, and only those get a - // decision recorded (a thread with no calendar part yet is simply - // re-checked next pass in case one arrives later). - const calendarMsgs = msgs.filter((m) => - (m.attachments ?? []).some((a) => isCalendarAttachment(a.mimeType)) + // Fetch every calendar part not yet examined. The cheap in-memory filter + // keeps threads with no calendar part off IMAP entirely, and `seenIcs` + // keeps a part that has already been read off it on every later pass + // within the rescan window. + const seen = new Set(entry?.seenIcs ?? []); + const icsByKey = new Map(); + const unexamined = msgs.filter( + (m) => + (m.attachments ?? []).some((a) => isCalendarAttachment(a.mimeType)) && + !seen.has(noteKeyOf(m)) ); - if (calendarMsgs.length === 0) continue; - let classified: ClassifiedICS | null = null; - for (const m of calendarMsgs) { + for (const m of unexamined) { const part = (m.attachments ?? []).find((a) => isCalendarAttachment(a.mimeType))!; - await host.imap.selectMailbox(session, m.mailbox); const bytes = await host.imap.fetchAttachment(session, m.uid, part.partNumber); const ics = new TextDecoder("utf-8").decode(bytes); + icsByKey.set(noteKeyOf(m), ics); + seen.add(noteKeyOf(m)); + } + + if (entry && unexamined.length > 0) { + entry.seenIcs = [...seen].slice(-SEEN_ICS_MAX); + changed.add(root); + } + + // Everything past here is the ONE-TIME bundling classification; a root + // that already has a decision keeps it. + if (persisted) continue; + + // Nothing to classify from: either no calendar part at all, or every part + // was read on an earlier pass (in which case a decision was recorded then + // and `persisted` already sent us round). Leave the root undecided so a + // part arriving on a later pass is still evaluated. + if (icsByKey.size === 0) continue; + + let classified: ClassifiedICS | null = null; + for (const m of msgs) { + const ics = icsByKey.get(noteKeyOf(m)); + if (!ics) continue; classified = classifyICS(ics); if (!classified) continue; // bare invite or RSVP — check the thread's other messages @@ -318,7 +387,6 @@ export async function detectCalendarBundles( // Record the decision — including explicit "no bundle" — so this root is // never re-evaluated on a later pass (see the caching doc above). - const entry = meta.get(root); if (entry) { entry.bundle = { classified }; changed.add(root); @@ -328,7 +396,7 @@ export async function detectCalendarBundles( bundles.set(root, { ...classified, eventKnown: await resolveEventKnown(classified.uid) }); } } - return bundles; + return { bundles, foldedNoteKeys }; } /** How one mailbox is being read this pass. */ @@ -619,6 +687,11 @@ export async function mailSync( nextMeta.set(root, { channelId, ...(prev?.bundle ? { bundle: prev.bundle } : {}), + // Carried forward, like `bundle`: this map is rebuilt from scratch + // every pass, so anything not copied here is lost, and a dropped + // `seenIcs` means every calendar part in the rescan window is + // re-fetched on every poll. + ...(prev?.seenIcs ? { seenIcs: prev.seenIcs } : {}), }); if (!prev || prev.channelId !== channelId) changedMeta.add(root); } @@ -634,7 +707,7 @@ export async function mailSync( initialRoots.add(root); } - const calendarBundles = await detectCalendarBundles( + const { bundles: calendarBundles } = await detectCalendarBundles( host, session, merged, diff --git a/connectors/apple/src/mail/transform.ts b/connectors/apple/src/mail/transform.ts index 24840867..9ad9f615 100644 --- a/connectors/apple/src/mail/transform.ts +++ b/connectors/apple/src/mail/transform.ts @@ -84,8 +84,13 @@ function compareMessages(a: MailMessage, b: MailMessage): number { return a.uid - b.uid; } -/** The note key for one message: its stripped Message-ID, else a uid fallback. */ -function noteKeyOf(m: MailMessage): string { +/** + * Stable per-message identity: the note `key` a message's note is written + * under. Exported because `sync.ts` records examined calendar parts and folded + * RSVPs under the SAME identity — a set keyed differently would never match + * the notes it is meant to filter. + */ +export function noteKeyOf(m: MailMessage): string { return m.messageId ? stripAngle(m.messageId) : `uid-${m.uid}`; } From ee3fdc8a194e2ab721e1c8513f71e58fc8a22068 Mon Sep 17 00:00:00 2001 From: Kris Braun Date: Mon, 3 Aug 2026 16:46:23 -0400 Subject: [PATCH 03/10] feat(apple): fold attendee responses onto the event's thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An "Accepted: " notification is an answer to an invitation, not correspondence of its own. The iCloud mail sync now routes every `METHOD:REPLY` calendar part through the shared response-folding rules instead of leaving it to become an email thread beside the event. A bare acceptance writes no note at all. That is the only way it can stop pulling the organiser's event thread back to unread: attaching a note is itself what surfaces a thread as unread for every recipient but its author, and no field passed to `saveNote` suppresses that. Everything that carries new information still gets a note on the event's thread — a decline or a tentative, an acceptance with a personal comment, and an acceptance that reverses an earlier decline. Each folded response is recorded per attendee and per occurrence, so a redelivered response is recognised rather than re-emitted, and a decline on one instance of a recurring meeting is never mistaken for an outstanding non-acceptance on another. Markers are flushed in one write before the pass saves its links, so a failure there can never leave a note on file with nothing recording it. Two mailbox copies of one response (a folder copy alongside INBOX) are now collapsed as the parts are read, rather than only later when the thread's messages are deduplicated — otherwise both copies read the same pre-pass marker and the response was emitted twice. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GcnBUEdW86ovpv1pT2d3b3 --- connectors/apple/src/mail/sync.test.ts | 236 +++++++++++++++++++++++++ connectors/apple/src/mail/sync.ts | 137 +++++++++++++- 2 files changed, 368 insertions(+), 5 deletions(-) diff --git a/connectors/apple/src/mail/sync.test.ts b/connectors/apple/src/mail/sync.test.ts index 2452626a..ac05006a 100644 --- a/connectors/apple/src/mail/sync.test.ts +++ b/connectors/apple/src/mail/sync.test.ts @@ -73,6 +73,16 @@ function buildFakeHost(opts: { /** One entry per `saveLinks()` INVOCATION — the merged pass must make * exactly one per sync, never one per mailbox. */ const saveLinksCalls: NewLinkWithNotes[][] = []; + /** Every note passed to `integrations.saveNote()`, in order. */ + const savedNotes: Record[] = []; + /** + * The ORDER in which the pass wrote: `"saveNote"`, `"setMany"`, + * `"saveLinks"`. Fold markers must be flushed before the links are saved, + * so a `saveLinks` that throws can never leave a note on file with no + * marker recording it — the next pass would re-emit that note and drag a + * thread people had already read back to unread. + */ + const callLog: string[] = []; const searchCalls: SearchCall[] = []; const fetchCalls: FetchCall[] = []; const fetchAttachmentCalls: FetchAttachmentCall[] = []; @@ -154,10 +164,16 @@ function buildFakeHost(opts: { const setThreadToDo = vi.fn(async () => {}); const integrations = { saveLinks: async (links: NewLinkWithNotes[]): Promise<(string | null)[]> => { + callLog.push("saveLinks"); saveLinksCalls.push(links); savedLinks.push(...links); return links.map(() => null); }, + saveNote: async (note: Record): Promise => { + callLog.push("saveNote"); + savedNotes.push(note); + return "note-id"; + }, setThreadToDo, } as unknown as Integrations; @@ -175,6 +191,7 @@ function buildFakeHost(opts: { stored.set(key, value); }, setMany: async (entries: [key: string, value: T][]): Promise => { + callLog.push("setMany"); setManyCalls.push(entries.map(([key]) => key)); for (const [key, value] of entries) stored.set(key, value); }, @@ -194,6 +211,8 @@ function buildFakeHost(opts: { stored, savedLinks, saveLinksCalls, + savedNotes, + callLog, searchCalls, fetchCalls, fetchAttachmentCalls, @@ -1937,6 +1956,223 @@ describe("detectCalendarBundles", () => { }); }); +const REPLY_UID = "evt-fold@example.test"; + +/** A `METHOD:REPLY` body in the shape both Google and Exchange emit. */ +function replyIcs(opts: { + partstat: "ACCEPTED" | "DECLINED" | "TENTATIVE"; + comment?: string; + recurrenceId?: string; +}): string { + return [ + "BEGIN:VCALENDAR", + "METHOD:REPLY", + "BEGIN:VEVENT", + `UID:${REPLY_UID}`, + ...(opts.recurrenceId ? [`RECURRENCE-ID:${opts.recurrenceId}`] : []), + `ATTENDEE;CN=Sam Guest;PARTSTAT=${opts.partstat}:mailto:guest@example.test`, + ...(opts.comment ? [`COMMENT:${opts.comment}`] : []), + "END:VEVENT", + "END:VCALENDAR", + ].join("\r\n"); +} + +/** The one reply message every fold fixture is built from. */ +function replyMessage(uid = 51): ImapMessage { + return calendarMessage({ + uid, + messageId: "", + root: "", + }); +} + +/** Run one `detectCalendarBundles` pass over a single reply message. */ +async function runFold( + ics: string, + opts: { stored?: Record } = {} +): Promise<{ + host: MailHost; + savedNotes: Record[]; + foldedOf: string[]; +}> { + const built = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [replyMessage()])], + attachments: { [buildAttachmentRef("INBOX", 51, "2")]: icsBytes(ics) }, + }); + for (const [k, v] of Object.entries(opts.stored ?? {})) built.stored.set(k, v); + + const messages: MailMessage[] = [{ ...replyMessage(), mailbox: "INBOX" }]; + const meta = new Map([["invite@example.test", { channelId: "INBOX" }]]); + const result = await detectCalendarBundles( + built.host, + "session-1", + messages, + meta, + new Set() + ); + return { + host: built.host, + savedNotes: built.savedNotes, + foldedOf: [...result.foldedNoteKeys], + }; +} + +describe("detectCalendarBundles — attendee responses", () => { + it("writes no note for a bare acceptance, and folds the message away", async () => { + const { host, savedNotes, foldedOf } = await runFold(replyIcs({ partstat: "ACCEPTED" })); + expect(savedNotes).toHaveLength(0); + expect(foldedOf).toContain("reply-1@example.test"); + // Nothing recorded: the marker tracks what was FOLDED onto the thread. + expect(await host.get(`rsvp:${REPLY_UID}:series:guest@example.test`)).toBeUndefined(); + }); + + it("writes a note for a decline, onto the event thread, and records it", async () => { + const { host, savedNotes } = await runFold(replyIcs({ partstat: "DECLINED" })); + expect(savedNotes).toHaveLength(1); + expect(savedNotes[0]).toMatchObject({ + thread: { source: `icaluid:${REPLY_UID}` }, + key: "reply-1@example.test", + content: "Sam Guest declined.", + contentType: "markdown", + deferUntilThread: true, + unread: true, + author: { email: "guest@example.test", name: "Sam Guest" }, + }); + expect(await host.get(`rsvp:${REPLY_UID}:series:guest@example.test`)).toBe("DECLINED"); + }); + + it("writes a note for an acceptance that carries a personal comment", async () => { + const { savedNotes } = await runFold( + replyIcs({ partstat: "ACCEPTED", comment: "Running 10 minutes late" }) + ); + expect(savedNotes).toHaveLength(1); + expect(savedNotes[0].content).toBe("Sam Guest accepted.\n\n> Running 10 minutes late"); + }); + + it("writes a note for an acceptance that reverses a decline, and updates the marker", async () => { + const { host, savedNotes } = await runFold(replyIcs({ partstat: "ACCEPTED" }), { + stored: { [`rsvp:${REPLY_UID}:series:guest@example.test`]: "DECLINED" }, + }); + expect(savedNotes).toHaveLength(1); + expect(savedNotes[0].content).toBe("Sam Guest accepted."); + // Now ACCEPTED, so a later repeat of this same acceptance is recognised + // by alreadyFolded rather than emitted again. + expect(await host.get(`rsvp:${REPLY_UID}:series:guest@example.test`)).toBe("ACCEPTED"); + }); + + it("writes no second note when the same response is redelivered", async () => { + const { savedNotes } = await runFold(replyIcs({ partstat: "DECLINED" }), { + stored: { [`rsvp:${REPLY_UID}:series:guest@example.test`]: "DECLINED" }, + }); + expect(savedNotes).toHaveLength(0); + }); + + it("scopes the marker per occurrence, so a decline on one instance does not mask another", async () => { + // A decline on the 4 Aug occurrence must NOT make a bare acceptance on + // the 11 Aug occurrence read as a reversal — that would re-open the + // original bug for every recurring meeting. + const { savedNotes } = await runFold( + replyIcs({ partstat: "ACCEPTED", recurrenceId: "20260811T140000Z" }), + { stored: { [`rsvp:${REPLY_UID}:2026-08-04T14:00:00.000Z:guest@example.test`]: "DECLINED" } } + ); + expect(savedNotes).toHaveLength(0); + }); + + it("keys the marker per occurrence across passes, so a decline on one instance never masks another", async () => { + // The seeded-marker test above cannot prove the scoping on its own: the + // SAME key expression both reads and writes, so dropping the occurrence + // from it moves both sides together and a pre-seeded occurrence-scoped + // marker simply stops matching — no note either way. Recording the 4 Aug + // decline through the real write path and only then answering the 11 Aug + // occurrence is what makes the scoping load-bearing: unscoped, the second + // pass reads the first pass's marker and takes a bare acceptance for a + // reversal, which is the original defect re-opened for every recurring + // meeting. + const decline = calendarMessage({ + uid: 51, + messageId: "", + root: "", + }); + const accept = calendarMessage({ + uid: 52, + messageId: "", + root: "", + }); + const built = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [decline, accept])], + attachments: { + [buildAttachmentRef("INBOX", 51, "2")]: icsBytes( + replyIcs({ partstat: "DECLINED", recurrenceId: "20260804T140000Z" }) + ), + [buildAttachmentRef("INBOX", 52, "2")]: icsBytes( + replyIcs({ partstat: "ACCEPTED", recurrenceId: "20260811T140000Z" }) + ), + }, + }); + + // Two passes, one message each — the marker written by the first is what + // the second reads. + for (const m of [decline, accept]) { + await detectCalendarBundles( + built.host, + "session-1", + [{ ...m, mailbox: "INBOX" }], + new Map([["invite@example.test", { channelId: "INBOX" }]]), + new Set() + ); + } + + expect(built.savedNotes).toHaveLength(1); + expect(built.savedNotes[0].content).toBe("Sam Guest declined the August 4, 2026 occurrence."); + }); + + it("names the occurrence in the note when the response was to one instance", async () => { + const { savedNotes } = await runFold( + replyIcs({ partstat: "DECLINED", recurrenceId: "20260811T140000Z" }) + ); + expect(savedNotes[0].content).toBe("Sam Guest declined the August 11, 2026 occurrence."); + }); + + it("emits ONE note when a merged pass holds two mailbox copies of the same response", async () => { + // A user who keeps a copy in a project folder as well as INBOX gives the + // pass two copies of one message. `dedupeCopies` collapses them, but only + // later, inside `transformMessages` — so this loop must collapse them + // itself. Both copies read the same (absent) marker, because the marker + // flush happens once at the end of the pass, so neither would be + // recognised as already folded and the note's unread intent would be + // applied twice: the exact thing the fold exists to prevent. + const built = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [replyMessage()]), box("Archive", [replyMessage()])], + // BOTH copies are fetchable, so a missing guard shows up as a second + // note rather than as a lookup miss throwing for an unrelated reason. + attachments: { + [buildAttachmentRef("INBOX", 51, "2")]: icsBytes(replyIcs({ partstat: "DECLINED" })), + [buildAttachmentRef("Archive", 51, "2")]: icsBytes(replyIcs({ partstat: "DECLINED" })), + }, + }); + const messages: MailMessage[] = [ + { ...replyMessage(), mailbox: "INBOX" }, + { ...replyMessage(), mailbox: "Archive" }, + ]; + const meta = new Map([["invite@example.test", { channelId: "INBOX" }]]); + + const { foldedNoteKeys } = await detectCalendarBundles( + built.host, + "session-1", + messages, + meta, + new Set() + ); + + expect(built.savedNotes).toHaveLength(1); + expect(built.fetchAttachmentCalls).toHaveLength(1); + expect([...foldedNoteKeys]).toEqual(["reply-1@example.test"]); + }); +}); + describe("mailSync — calendar thread bundling end-to-end", () => { it("bundles a CANCEL invite onto an ALREADY-SYNCED event's thread and omits its title key", async () => { const cancelMsg = msg({ diff --git a/connectors/apple/src/mail/sync.ts b/connectors/apple/src/mail/sync.ts index 51b59905..254b3672 100644 --- a/connectors/apple/src/mail/sync.ts +++ b/connectors/apple/src/mail/sync.ts @@ -1,3 +1,12 @@ +import { + alreadyFolded, + composeRsvpNote, + icsProp, + isNonAcceptance, + parseIcsReply, + priorRsvpKey, + shouldEmitRsvpNote, +} from "@plotday/rsvp-fold"; import type { ActorId } from "@plotday/twister"; import type { ImapMailboxStatus, ImapSession } from "@plotday/twister/tools/imap"; @@ -279,10 +288,19 @@ export async function reconcileTodoFlags( * was settled by the pass which ingested the invitation, and gating the whole * root on the cache would mean never looking at them. * + * ATTENDEE RESPONSES: a `METHOD:REPLY` part is an answer to an invitation, so + * it belongs on the event's own thread rather than in a mail thread of its + * own. Each one is routed through `@plotday/rsvp-fold` — which decides whether + * the response says anything the event's guest list does not already show — + * and its message is reported in `foldedNoteKeys` either way, so the caller + * can leave it out of the mail thread's notes. A bare acceptance gets NO note + * at all: attaching a note is what surfaces a thread as unread for every + * recipient but its author, and no field passed to `saveNote` suppresses that, + * so writing nothing is the only way an acceptance stops pulling the + * organiser's event thread back to unread. + * * Returns the per-root `bundles` map plus `foldedNoteKeys`: note keys of - * messages this function consumed itself, so the caller can leave them out of - * the thread's notes. Nothing is consumed yet — the set is always empty — but - * callers are written against the final shape. + * messages this function consumed itself. * * The caller owns the store I/O: `meta` arrives pre-loaded (one read per root * for the pass, shared with home-channel resolution) and is MUTATED in place @@ -294,16 +312,25 @@ export async function reconcileTodoFlags( * `eventKnown` (see `CalendarBundle`'s doc) is resolved via * `host.knownEventUids()` at most once per call — lazily, only once a * bundle is actually found — never per message/thread. + * + * `initialRoots` carries `mailSync`'s per-root initial-ness (see there) into + * the fold, so a response ingested from history on first connect is attached + * without marking the event thread unread — the same discipline + * `transformMessages` applies to the mail it ingests. It defaults to empty so + * a caller that has no such notion still gets the safe, live-mail behaviour. */ export async function detectCalendarBundles( host: MailHost, session: ImapSession, messages: MailMessage[], meta: Map, - changed: Set + changed: Set, + initialRoots: Set = new Set() ): Promise<{ bundles: Map; foldedNoteKeys: Set }> { const bundles = new Map(); const foldedNoteKeys = new Set(); + /** Fold markers to persist, flushed in ONE `setMany` before returning. */ + const rsvpMarkers: [string, string][] = []; let knownUids: Set | null = null; const resolveEventKnown = async (uid: string): Promise => { if (knownUids === null) knownUids = await host.knownEventUids(); @@ -349,10 +376,93 @@ export async function detectCalendarBundles( ); for (const m of unexamined) { + // A merged pass can hold two mailbox copies of ONE message (a copy kept + // in a project folder as well as INBOX). They share a note key, and + // `dedupeCopies` only collapses them later, inside `transformMessages` — + // `unexamined` was filtered against a snapshot of `seen`, so both copies + // are in it. The second copy has nothing new to read, and routing it + // again would emit its response note twice: the marker written below is + // flushed once at the end of the pass, so the second copy would still + // read the pre-pass value and look un-folded. + if (seen.has(noteKeyOf(m))) continue; + const part = (m.attachments ?? []).find((a) => isCalendarAttachment(a.mimeType))!; await host.imap.selectMailbox(session, m.mailbox); const bytes = await host.imap.fetchAttachment(session, m.uid, part.partNumber); const ics = new TextDecoder("utf-8").decode(bytes); + + // An attendee response is routed here, BEFORE the part is offered to the + // bundling classifier below: `classifyICS` returns null for a REPLY, so + // a reply that fell through would have the root recorded as "evaluated, + // does not bundle" on the strength of a message that answers a question + // it was never asked — and that decision is permanent, so a real invite + // arriving on the same root later would never be classified. + const reply = parseIcsReply(ics, { name: m.from?.[0]?.name ?? null }); + if (reply) { + // The event being answered. `parseIcsReply` reads the ATTENDEE line, + // not the event id, so the UID is read from the same body here. + // Without one there is no thread to address and no way to scope the + // marker, so such a response is left as ordinary mail rather than + // folded into nowhere — but it is still never classified, since a + // reply is not an answer to the bundling question. + const replyUid = icsProp(ics, "UID"); + if (replyUid) { + const priorKey = priorRsvpKey(replyUid, reply.attendeeEmail, reply.occurrence); + const stored = await host.get(priorKey); + + // Order is the library's documented contract: `alreadyFolded` + // FIRST, and only when it is false decide whether to emit. + // Re-emitting a note the thread already carries re-applies its + // unread intent and drags the thread back to unread for everyone + // who had read it — and a response inside the 30-day rescan window + // is re-read on every pass. + if ( + !alreadyFolded(stored, reply) && + shouldEmitRsvpNote(reply, isNonAcceptance(stored)) + ) { + // `saveNote` returns null when no thread carries `icaluid:` + // yet (the calendar event has not synced); `deferUntilThread` has + // the platform hold the note and attach it once that thread + // appears. + await host.integrations.saveNote({ + thread: { source: `icaluid:${replyUid}` }, + key: noteKeyOf(m), + content: composeRsvpNote(reply), + contentType: "markdown", + ...(m.date ? { created: m.date } : {}), + author: { + email: reply.attendeeEmail, + ...(reply.attendeeName ? { name: reply.attendeeName } : {}), + }, + // Explicit on both paths. An omitted flag does NOT mean "leave + // read state alone" — attaching a note already marks the thread + // unread for every recipient except its author, so only an + // explicit false overrides it. + unread: !initialRoots.has(root), + deferUntilThread: true, + }); + // Recorded ONLY on the path that emits, and regardless of the + // return value: a deferred note returns no id, and gating on it + // would leave a deferred non-acceptance unrecorded forever, + // wrongly treating a later bare acceptance as reversing nothing. + // The marker holds the last response actually folded onto the + // thread — for every emitted response, acceptances included, + // which is what lets `alreadyFolded` recognise a repeat of ANY + // partstat. + rsvpMarkers.push([priorKey, reply.partstat]); + } + + // Folded whether or not a note was written — a bare acceptance is + // dropped from the mail thread rather than left to become an email + // thread of its own. + foldedNoteKeys.add(noteKeyOf(m)); + } + + // Deliberately NOT added to `icsByKey`: see the comment above. + seen.add(noteKeyOf(m)); + continue; + } + icsByKey.set(noteKeyOf(m), ics); seen.add(noteKeyOf(m)); } @@ -396,6 +506,19 @@ export async function detectCalendarBundles( bundles.set(root, { ...classified, eventKnown: await resolveEventKnown(classified.uid) }); } } + + // ONE `setMany`, per `MailHost.setMany`'s contract: a pass can fold many + // responses and a `set` each would burn a request apiece. + // + // Written HERE — before `mailSync`'s `saveLinks` — deliberately, and + // opposite to `ThreadMeta`, which is persisted AFTER `saveLinks` so a throw + // re-runs the initial-sync discipline. The two fail in opposite directions: + // a marker written after a throwing `saveLinks` would leave the note on file + // with nothing recording it, so the next pass would re-emit it and re-raise + // unread on a thread people had already read. Do not "tidy" these into one + // place. + if (rsvpMarkers.length > 0) await host.setMany(rsvpMarkers); + return { bundles, foldedNoteKeys }; } @@ -707,12 +830,16 @@ export async function mailSync( initialRoots.add(root); } + // `initialRoots` is what keeps a first-connect backfill quiet: a response + // folded onto an event thread from history must not mark it unread, the + // same discipline `transformMessages` applies to the mail it ingests. const { bundles: calendarBundles } = await detectCalendarBundles( host, session, merged, nextMeta, - changedMeta + changedMeta, + initialRoots ); // THE single transformMessages call. See this function's docstring. From 962eef25ad6c77e52f4f57e72fc757ecb8403413 Mon Sep 17 00:00:00 2001 From: Kris Braun Date: Mon, 3 Aug 2026 16:56:54 -0400 Subject: [PATCH 04/10] test(apple): cover the first-connect path for folded responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule that a response ingested from history must not mark the event thread unread had no test: every fold test drove the pass with an empty initial-roots set, so the branch was unreachable and replacing it with an unconditional "unread" left the suite green. The fold driver can now put the reply's thread root into that set, and a test asserts the saved note carries `unread: false` — the same discipline the mail transform already applies to the messages it ingests. Also drops an occurrence-scoping test that could not fail. The marker key is built by one expression used for both the read and the write, so a single pass with a pre-seeded marker cannot detect an unscoped key: both sides move together and the seeded marker simply stops matching. The two-pass test that records one occurrence's decline before answering another is the one that holds the rule, and its comment now carries that reasoning. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GcnBUEdW86ovpv1pT2d3b3 --- connectors/apple/src/mail/sync.test.ts | 56 +++++++++++++++----------- connectors/apple/src/mail/sync.ts | 6 +++ 2 files changed, 38 insertions(+), 24 deletions(-) diff --git a/connectors/apple/src/mail/sync.test.ts b/connectors/apple/src/mail/sync.test.ts index ac05006a..13aeeeb7 100644 --- a/connectors/apple/src/mail/sync.test.ts +++ b/connectors/apple/src/mail/sync.test.ts @@ -1986,10 +1986,15 @@ function replyMessage(uid = 51): ImapMessage { }); } -/** Run one `detectCalendarBundles` pass over a single reply message. */ +/** + * Run one `detectCalendarBundles` pass over a single reply message. + * + * `initial` puts the reply's thread root into `initialRoots`, i.e. this pass is + * ingesting it from history rather than receiving it as live mail. + */ async function runFold( ics: string, - opts: { stored?: Record } = {} + opts: { stored?: Record; initial?: boolean } = {} ): Promise<{ host: MailHost; savedNotes: Record[]; @@ -2009,7 +2014,8 @@ async function runFold( "session-1", messages, meta, - new Set() + new Set(), + new Set(opts.initial ? ["invite@example.test"] : []) ); return { host: built.host, @@ -2042,6 +2048,18 @@ describe("detectCalendarBundles — attendee responses", () => { expect(await host.get(`rsvp:${REPLY_UID}:series:guest@example.test`)).toBe("DECLINED"); }); + it("does not mark the event thread unread for a response ingested from history", async () => { + // First connect backfills whatever history the plan grants, and every + // response in it would otherwise light up the event thread. Same + // discipline `transformMessages` applies to the mail it ingests — and the + // sole reason `initialRoots` is threaded through this function at all. + const { savedNotes } = await runFold(replyIcs({ partstat: "DECLINED" }), { + initial: true, + }); + expect(savedNotes).toHaveLength(1); + expect(savedNotes[0].unread).toBe(false); + }); + it("writes a note for an acceptance that carries a personal comment", async () => { const { savedNotes } = await runFold( replyIcs({ partstat: "ACCEPTED", comment: "Running 10 minutes late" }) @@ -2068,27 +2086,17 @@ describe("detectCalendarBundles — attendee responses", () => { expect(savedNotes).toHaveLength(0); }); - it("scopes the marker per occurrence, so a decline on one instance does not mask another", async () => { - // A decline on the 4 Aug occurrence must NOT make a bare acceptance on - // the 11 Aug occurrence read as a reversal — that would re-open the - // original bug for every recurring meeting. - const { savedNotes } = await runFold( - replyIcs({ partstat: "ACCEPTED", recurrenceId: "20260811T140000Z" }), - { stored: { [`rsvp:${REPLY_UID}:2026-08-04T14:00:00.000Z:guest@example.test`]: "DECLINED" } } - ); - expect(savedNotes).toHaveLength(0); - }); - - it("keys the marker per occurrence across passes, so a decline on one instance never masks another", async () => { - // The seeded-marker test above cannot prove the scoping on its own: the - // SAME key expression both reads and writes, so dropping the occurrence - // from it moves both sides together and a pre-seeded occurrence-scoped - // marker simply stops matching — no note either way. Recording the 4 Aug - // decline through the real write path and only then answering the 11 Aug - // occurrence is what makes the scoping load-bearing: unscoped, the second - // pass reads the first pass's marker and takes a bare acceptance for a - // reversal, which is the original defect re-opened for every recurring - // meeting. + it("keys the marker per occurrence, so a decline on one instance never masks another", async () => { + // Both passes are needed. A single pass with a PRE-SEEDED marker proves + // nothing here: the same key expression both reads and writes, so dropping + // the occurrence from it moves both sides together and the seeded + // occurrence-scoped marker simply stops matching — no note either way, and + // the test passes just as happily against the broken code. Recording the + // 4 Aug decline through the real write path and only then answering the + // 11 Aug occurrence is what makes the scoping load-bearing: unscoped, the + // second pass reads the first pass's marker and takes a bare acceptance + // for a reversal, which is the original defect re-opened for every + // recurring meeting. const decline = calendarMessage({ uid: 51, messageId: "", diff --git a/connectors/apple/src/mail/sync.ts b/connectors/apple/src/mail/sync.ts index 254b3672..9843a1a9 100644 --- a/connectors/apple/src/mail/sync.ts +++ b/connectors/apple/src/mail/sync.ts @@ -517,6 +517,12 @@ export async function detectCalendarBundles( // with nothing recording it, so the next pass would re-emit it and re-raise // unread on a thread people had already read. Do not "tidy" these into one // place. + // + // Batching costs one thing a write per note would not: if a later `saveNote` + // in this pass throws, NO markers are written — including for responses + // already emitted before it, which are then re-emitted on the next pass. + // Accepted for the request budget, and it fails in the same direction as + // everything else here: a repeated note, never a missing one. if (rsvpMarkers.length > 0) await host.setMany(rsvpMarkers); return { bundles, foldedNoteKeys }; From eaa67f0f01887a3964cb330dcb1dc1524750da58 Mon Sep 17 00:00:00 2001 From: Kris Braun Date: Mon, 3 Aug 2026 17:05:08 -0400 Subject: [PATCH 05/10] feat(apple): keep folded responses out of the mail thread Attendee acceptances/declines that get folded onto a calendar event's thread should not also show up as their own row in ordinary mail. transformMessages now drops the note for any message whose key is in the folded set, and skips emitting a link entirely when every message in a thread was folded (otherwise the thread would surface as a titled row with no content). --- connectors/apple/src/mail/sync.ts | 3 +- connectors/apple/src/mail/transform.test.ts | 49 ++++++++++++++++++ connectors/apple/src/mail/transform.ts | 55 ++++++++++++++------- 3 files changed, 87 insertions(+), 20 deletions(-) diff --git a/connectors/apple/src/mail/sync.ts b/connectors/apple/src/mail/sync.ts index 9843a1a9..186353ce 100644 --- a/connectors/apple/src/mail/sync.ts +++ b/connectors/apple/src/mail/sync.ts @@ -839,7 +839,7 @@ export async function mailSync( // `initialRoots` is what keeps a first-connect backfill quiet: a response // folded onto an event thread from history must not mark it unread, the // same discipline `transformMessages` applies to the mail it ingests. - const { bundles: calendarBundles } = await detectCalendarBundles( + const { bundles: calendarBundles, foldedNoteKeys } = await detectCalendarBundles( host, session, merged, @@ -856,6 +856,7 @@ export async function mailSync( newMessages, sentMailbox: sentBox, calendarBundles, + foldedNoteKeys, }); if (links.length > 0) await host.integrations.saveLinks(links); diff --git a/connectors/apple/src/mail/transform.test.ts b/connectors/apple/src/mail/transform.test.ts index 660776f3..efbcb21f 100644 --- a/connectors/apple/src/mail/transform.test.ts +++ b/connectors/apple/src/mail/transform.test.ts @@ -983,3 +983,52 @@ describe("transformMessages — signals", () => { // test suites, which dropped their equivalent CTA cases outright rather // than replacing them with a signal assertion. }); + +describe("transformMessages — folded attendee responses", () => { + const ROOT = ""; + + it("drops a folded message's note but keeps the rest of the conversation", () => { + const real = msg({ + uid: 1, + messageId: "", + references: [ROOT], + bodyText: "Can we move this?", + }); + const rsvp = msg({ + uid: 2, + messageId: "", + references: [ROOT], + bodyText: "Sam declined.", + }); + + const links = transform([real, rsvp], { + foldedNoteKeys: new Set(["rsvp@example.test"]), + }); + + expect(links).toHaveLength(1); + expect(links[0].notes!.map((n) => (n as { key: string }).key)).toEqual(["real@example.test"]); + }); + + it("emits NO link at all for a thread that was nothing but responses", () => { + // Without this guard the thread becomes a titled email link with zero + // notes — an empty row in the user's list for a response that has + // already been folded onto the event. + const rsvp = msg({ + uid: 2, + messageId: "", + references: [ROOT], + bodyText: "Sam declined.", + }); + + const links = transform([rsvp], { foldedNoteKeys: new Set(["rsvp@example.test"]) }); + + expect(links).toEqual([]); + }); + + it("is unaffected when nothing was folded", () => { + const real = msg({ uid: 1, messageId: "", references: [ROOT] }); + const links = transform([real]); + expect(links).toHaveLength(1); + expect(links[0].notes).toHaveLength(1); + }); +}); diff --git a/connectors/apple/src/mail/transform.ts b/connectors/apple/src/mail/transform.ts index 9ad9f615..3063d384 100644 --- a/connectors/apple/src/mail/transform.ts +++ b/connectors/apple/src/mail/transform.ts @@ -253,6 +253,17 @@ export type TransformCtx = { * `@plotday/twister/plot`. */ calendarBundles?: Map; + /** + * Note keys whose messages were folded onto a calendar event's thread by + * `sync.ts`'s `detectCalendarBundles`. Their notes are dropped here so an + * attendee response does not ALSO appear as ordinary mail. + * + * Filtered at the NOTES level rather than by removing the messages from the + * input: `allCopies` drives the Sent-only rule and dedupe, and removing an + * inbound response from it could make a mixed thread look Sent-only and + * change its read-state handling. + */ + foldedNoteKeys?: Set; }; function toContact(a: ImapAddress): NewContact { @@ -386,25 +397,31 @@ export function transformMessages( } } - const notes = msgs.map((m) => { - const key = noteKeyOf(m); - const body = bodyOf(m); - const from = m.from && m.from[0] ? m.from[0] : null; - const isOwner = from?.address.toLowerCase() === ownEmail; - const actions = attachmentActions(m); - return { - key, - content: body?.content ?? "", - contentType: body?.contentType ?? ("text" as const), - created: m.date, - // Owner's own messages: credit via authoredBySelf, leave author unset. - ...(isOwner - ? { authoredBySelf: true as const } - : { author: from ? toContact(from) : null }), - ...(actions ? { actions } : {}), - accessContacts: messageContacts(m, ownEmail), - }; - }); + const notes = msgs + .filter((m) => !ctx.foldedNoteKeys?.has(noteKeyOf(m))) + .map((m) => { + const key = noteKeyOf(m); + const body = bodyOf(m); + const from = m.from && m.from[0] ? m.from[0] : null; + const isOwner = from?.address.toLowerCase() === ownEmail; + const actions = attachmentActions(m); + return { + key, + content: body?.content ?? "", + contentType: body?.contentType ?? ("text" as const), + created: m.date, + // Owner's own messages: credit via authoredBySelf, leave author unset. + ...(isOwner + ? { authoredBySelf: true as const } + : { author: from ? toContact(from) : null }), + ...(actions ? { actions } : {}), + accessContacts: messageContacts(m, ownEmail), + }; + }); + + // Every message in this thread was folded onto a calendar event. Emitting + // the link anyway would create a titled row with no content. + if (notes.length === 0) continue; // Incremental read-state (see TransformCtx.newMessages): // - every message seen → mark read (a read done in Apple Mail) From f2adbdc1459c31885c9dd368f6ce12077a9769af Mon Sep 17 00:00:00 2001 From: Kris Braun Date: Mon, 3 Aug 2026 17:19:57 -0400 Subject: [PATCH 06/10] fix(apple): describe a mail thread from its earliest unfolded message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An attendee response folded onto a calendar event's thread no longer carries a note on the mail thread, but the mail thread was still described by whichever message came first overall. A conversation whose earliest in-window message was a response was therefore titled "Accepted: ", authored to the responder instead of to whoever started the conversation, and pointed `signals.noteKey` at a note that is not on the link — so body-derived classification fell back to some other message. Title, author and `signals.noteKey` now come from the earliest message that still carries a note. The participant union, read state and Sent-only rule are unchanged: a responder remains a participant of the conversation whether or not their message is shown. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GcnBUEdW86ovpv1pT2d3b3 --- connectors/apple/src/mail/transform.test.ts | 44 ++++++++++++ connectors/apple/src/mail/transform.ts | 79 +++++++++++++-------- 2 files changed, 93 insertions(+), 30 deletions(-) diff --git a/connectors/apple/src/mail/transform.test.ts b/connectors/apple/src/mail/transform.test.ts index efbcb21f..81fc793d 100644 --- a/connectors/apple/src/mail/transform.test.ts +++ b/connectors/apple/src/mail/transform.test.ts @@ -1025,6 +1025,50 @@ describe("transformMessages — folded attendee responses", () => { expect(links).toEqual([]); }); + it("describes the thread from its earliest SURVIVING message, not from a folded response", () => { + // A response can arrive before any of the conversation's real + // correspondence is in the window — a guest declines an invitation, and + // only later does someone reply about rescheduling. The response has no + // note on this thread, so describing the thread with it would title the + // thread "Accepted: Weekly sync", credit it to the responder, and point + // `signals.noteKey` at a note the link does not carry. + const rsvp = msg({ + uid: 1, + messageId: "", + references: [ROOT], + subject: "Accepted: Weekly sync", + from: [{ address: "guest@example.test", name: "Sam Guest" }], + date: new Date("2026-07-15T09:00:00Z"), + bodyText: "Sam Guest has accepted this invitation.", + }); + const real = msg({ + uid: 2, + messageId: "", + references: [ROOT], + subject: "Re: Weekly sync", + from: [{ address: "jane@example.test", name: "Jane" }], + date: new Date("2026-07-15T10:00:00Z"), + bodyText: "Can we move this?", + }); + + const links = transform([rsvp, real], { + foldedNoteKeys: new Set(["rsvp@example.test"]), + }); + + expect(links).toHaveLength(1); + expect(links[0].title).toBe("Re: Weekly sync"); + expect((links[0].author as { email?: string } | null)?.email).toBe("jane@example.test"); + expect(links[0].signals?.noteKey).toBe("real@example.test"); + // The classification pointer must name a note that is on the link. + expect((links[0].notes ?? []).map((n) => (n as { key?: string }).key)).toContain( + links[0].signals?.noteKey + ); + // The responder is still a participant of the conversation. + expect( + (links[0].accessContacts ?? []).map((c) => (c as { email?: string }).email) + ).toContain("guest@example.test"); + }); + it("is unaffected when nothing was folded", () => { const real = msg({ uid: 1, messageId: "", references: [ROOT] }); const links = transform([real]); diff --git a/connectors/apple/src/mail/transform.ts b/connectors/apple/src/mail/transform.ts index 3063d384..228bcd8c 100644 --- a/connectors/apple/src/mail/transform.ts +++ b/connectors/apple/src/mail/transform.ts @@ -256,7 +256,10 @@ export type TransformCtx = { /** * Note keys whose messages were folded onto a calendar event's thread by * `sync.ts`'s `detectCalendarBundles`. Their notes are dropped here so an - * attendee response does not ALSO appear as ordinary mail. + * attendee response does not ALSO appear as ordinary mail, and they are + * skipped when choosing the message the thread is described by — see the + * `surviving` set below, which drives `title`, `author` and + * `signals.noteKey`. * * Filtered at the NOTES level rather than by removing the messages from the * input: `allCopies` drives the Sent-only rule and dedupe, and removing an @@ -317,8 +320,9 @@ export function bodyOf(msg: ImapMessage): { content: string; contentType: "html" /** * Group a batch of messages by thread root and build one NewLinkWithNotes per * thread. Notes are keyed by (stripped) Message-ID for idempotent upsert; the - * link author is the earliest message's sender; accessContacts is the union of - * every participant seen; the owner's own messages are credited via + * link author is the earliest message's sender (earliest that still carries a + * note here — see `TransformCtx.foldedNoteKeys`); accessContacts is the union + * of every participant seen; the owner's own messages are credited via * authoredBySelf. * * `messages` must be the COMPLETE visible message set for every thread it @@ -372,8 +376,29 @@ export function transformMessages( // depends on which mailbox the merged pass happened to fetch first. const msgs = dedupeCopies(allCopies, homeMailbox).sort(compareMessages); - // Earliest message drives the thread's title + author. - const originator = msgs[0]; + // The messages that will actually carry a note on this thread. A folded + // attendee response (see `TransformCtx.foldedNoteKeys`) has been attached + // to the event's own thread and has no note here, so it must not be the + // message the thread is described by: a conversation whose earliest + // in-window message is a response notification would otherwise be titled + // "Accepted: ", credited to the responder rather than to whoever + // started the conversation, and have `signals.noteKey` point at a note + // that is not on the link at all — leaving body-derived classification to + // fall back to some other message. + // + // Only the description is affected: the participant union, the read state + // and the Sent-only rule all still consider every copy of every message, + // because a responder is a real participant in the conversation whether or + // not their message is shown here. + const surviving = msgs.filter((m) => !ctx.foldedNoteKeys?.has(noteKeyOf(m))); + + // Every message in this thread was folded onto a calendar event. Emitting + // the link anyway would create a titled row with no content. Checked + // before anything reads `surviving[0]`. + if (surviving.length === 0) continue; + + // Earliest surviving message drives the thread's title + author. + const originator = surviving[0]; const originatorFrom = originator.from && originator.from[0] ? originator.from[0] : null; // Mail signals are computed from the ORIGINATING message only (same @@ -397,31 +422,25 @@ export function transformMessages( } } - const notes = msgs - .filter((m) => !ctx.foldedNoteKeys?.has(noteKeyOf(m))) - .map((m) => { - const key = noteKeyOf(m); - const body = bodyOf(m); - const from = m.from && m.from[0] ? m.from[0] : null; - const isOwner = from?.address.toLowerCase() === ownEmail; - const actions = attachmentActions(m); - return { - key, - content: body?.content ?? "", - contentType: body?.contentType ?? ("text" as const), - created: m.date, - // Owner's own messages: credit via authoredBySelf, leave author unset. - ...(isOwner - ? { authoredBySelf: true as const } - : { author: from ? toContact(from) : null }), - ...(actions ? { actions } : {}), - accessContacts: messageContacts(m, ownEmail), - }; - }); - - // Every message in this thread was folded onto a calendar event. Emitting - // the link anyway would create a titled row with no content. - if (notes.length === 0) continue; + const notes = surviving.map((m) => { + const key = noteKeyOf(m); + const body = bodyOf(m); + const from = m.from && m.from[0] ? m.from[0] : null; + const isOwner = from?.address.toLowerCase() === ownEmail; + const actions = attachmentActions(m); + return { + key, + content: body?.content ?? "", + contentType: body?.contentType ?? ("text" as const), + created: m.date, + // Owner's own messages: credit via authoredBySelf, leave author unset. + ...(isOwner + ? { authoredBySelf: true as const } + : { author: from ? toContact(from) : null }), + ...(actions ? { actions } : {}), + accessContacts: messageContacts(m, ownEmail), + }; + }); // Incremental read-state (see TransformCtx.newMessages): // - every message seen → mark read (a read done in Apple Mail) From 6c69b8d64d15af7f172cec647186d59763b715a3 Mon Sep 17 00:00:00 2001 From: Kris Braun Date: Mon, 3 Aug 2026 17:20:08 -0400 Subject: [PATCH 07/10] test(apple): cover response folding across passes and write ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two things only a full sync pass can show: - A response is folded even when it arrives on a LATER pass than the invitation it threads onto. Calendar systems thread response notifications onto the invitation's Message-ID, so the response lands on a thread root the earlier pass already classified — it is only looked at because a settled root keeps being examined for messages it has not read. - Fold markers are durable before links are saved. If the link save fails after a response note was written, the marker recording that fold must already be on file, or the next pass re-emits the note and drags a thread people had read back to unread. The ordering assertion identifies the marker write by the keys it carried: a pass makes two batched store writes, and the other one — the per-thread metadata — is written after the save on purpose, so a positional match would silently assert about the wrong one. Also documents the fold in the calendar-part classifier's module doc, which still described a `METHOD:REPLY` only as "skip". Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GcnBUEdW86ovpv1pT2d3b3 --- connectors/apple/src/mail/calendar-bundle.ts | 38 ++-- connectors/apple/src/mail/sync.test.ts | 200 ++++++++++++++++++- 2 files changed, 224 insertions(+), 14 deletions(-) diff --git a/connectors/apple/src/mail/calendar-bundle.ts b/connectors/apple/src/mail/calendar-bundle.ts index 335dcafb..c54e7c4b 100644 --- a/connectors/apple/src/mail/calendar-bundle.ts +++ b/connectors/apple/src/mail/calendar-bundle.ts @@ -1,12 +1,22 @@ /** - * Mail-side half of mail↔calendar thread bundling (see `apple.ts`'s - * `buildEventSources()` for the calendar side, which already emits - * `["apple-calendar:", "icaluid:"]`). When an inbound email - * carries a `text/calendar`/`application/ics` MIME part, this classifies its + * Mail-side classification of a calendar MIME part, and the bundling half of + * how an email meets its event (see `apple.ts`'s `buildEventSources()` for the + * calendar side, which already emits + * `["apple-calendar:", "icaluid:"]`). When an inbound email carries + * a `text/calendar`/`application/ics` MIME part, this classifies its * relationship to the referenced event so `sync.ts` can decide whether to * bundle the mail thread onto the same Plot thread as the calendar event via * the shared `icaluid:` alias. * + * Bundling is not the only way a calendar part reaches the event's thread. + * `sync.ts` routes an attendee response (`METHOD:REPLY`) to a separate FOLD: + * the response is attached to the event's thread as a note of its own — or, if + * it is a bare acceptance saying nothing the guest list does not already show, + * dropped entirely — and its message is kept out of the mail thread either + * way. A response never reaches `classifyICS` in a sync pass, so the + * non-bundling verdict this file gives one says nothing about what becomes of + * the message. + * * Ports the Google connector's `classifyCalendarThread` decision * (`google/src/mail/gmail-api.ts`) — the product-approved rule for which ICS * methods bundle vs. skip — adapted to a single already-fetched ICS blob @@ -45,16 +55,18 @@ export function isCalendarAttachment(mimeType: string): boolean { * Classify one ICS (VCALENDAR) text's relationship to its event, per the * product-approved rule (see module doc): * - * | ICS content | Action | - * |-------------------------------------------|---------| - * | `METHOD:CANCEL` | bundle | - * | `METHOD:REQUEST` with `SEQUENCE > 0` | bundle | - * | `METHOD:REQUEST` with `SEQUENCE == 0` | skip | - * | `METHOD:REPLY` (an RSVP) | skip | + * | ICS content | Action | + * |--------------------------------------|--------------------------------------------| + * | `METHOD:CANCEL` | bundle | + * | `METHOD:REQUEST` with `SEQUENCE > 0` | bundle | + * | `METHOD:REQUEST` with `SEQUENCE == 0`| skip | + * | `METHOD:REPLY` (an RSVP) | folded onto the event thread (see sync.ts) | * - * Returns `null` for "skip" (including no parseable UID at all) so callers - * can uniformly treat every non-bundling case — RSVP, bare invite, or - * unparseable text — the same way. + * Returns `null` for everything that does not bundle (including no parseable + * UID at all) so callers can uniformly treat every non-bundling case — RSVP, + * bare invite, or unparseable text — the same way. A `METHOD:REPLY` still + * returns `null` here, but in a sync pass it is folded before this function is + * ever offered the part, so that `null` is only reachable from another caller. */ export function classifyICS(ics: string): ClassifiedICS | null { const uid = icsProp(ics, "UID"); diff --git a/connectors/apple/src/mail/sync.test.ts b/connectors/apple/src/mail/sync.test.ts index 13aeeeb7..8b74ef21 100644 --- a/connectors/apple/src/mail/sync.test.ts +++ b/connectors/apple/src/mail/sync.test.ts @@ -67,6 +67,13 @@ function buildFakeHost(opts: { * synced yet), which is the common case for these mail-only fixtures. */ knownEventUids?: string[]; + /** + * Make `saveLinks` record its call and then throw, as a platform write + * that fails would. `mailSync` has no catch around it, so the throw leaves + * the pass — everything written BEFORE the save is exactly what survives, + * which is what the fold-marker ordering exists to control. + */ + failSaveLinks?: boolean; }) { const stored = new Map(); const savedLinks: NewLinkWithNotes[] = []; @@ -96,6 +103,11 @@ function buildFakeHost(opts: { const mailboxes = new Map(); for (const box of opts.mailboxes) mailboxes.set(box.name, box); + /** Mutable so a test can add an attachment between two passes (see + * `addMessage`), which `opts.attachments` alone could not express when the + * caller passed none to begin with. */ + const attachments: Record = { ...(opts.attachments ?? {}) }; + const imap = { connect: async (): Promise => "session-1", listMailboxes: async (): Promise => @@ -155,7 +167,7 @@ function buildFakeHost(opts: { ): Promise => { fetchAttachmentCalls.push({ mailbox: selected, uid, partNumber }); const key = buildAttachmentRef(selected, uid, partNumber); - const bytes = opts.attachments?.[key]; + const bytes = attachments[key]; if (!bytes) throw new Error(`no such attachment part: ${key}`); return bytes; }, @@ -166,6 +178,7 @@ function buildFakeHost(opts: { saveLinks: async (links: NewLinkWithNotes[]): Promise<(string | null)[]> => { callLog.push("saveLinks"); saveLinksCalls.push(links); + if (opts.failSaveLinks) throw new Error("saveLinks failed"); savedLinks.push(...links); return links.map(() => null); }, @@ -209,6 +222,12 @@ function buildFakeHost(opts: { return { host, stored, + // The live mailbox fixtures and attachment bytes, so a test can deliver a + // message BETWEEN two passes over the same host (see `addMessage`) — + // which is the only way to exercise a response arriving after the + // invitation it threads onto. + mailboxes, + attachments, savedLinks, saveLinksCalls, savedNotes, @@ -319,6 +338,23 @@ function box( }; } +/** + * Deliver a message into a live mailbox fixture between two passes, as a + * server would: it becomes searchable, fetchable, and moves UIDNEXT. Mutating + * the fixture in place (rather than rebuilding the host) is what keeps the + * second pass reading the FIRST pass's stored cursors and thread metadata. + */ +function addMessage(fixture: MailboxFixture, message: ImapMessage): void { + fixture.messagesByUid.set(message.uid, message); + fixture.searchUids = [...fixture.searchUids, message.uid]; + fixture.status = { + ...fixture.status, + exists: fixture.messagesByUid.size, + uidNext: Math.max(fixture.status.uidNext, message.uid + 1), + unseen: [...fixture.messagesByUid.values()].filter((m) => !m.flags.includes("\\Seen")).length, + }; +} + /** A search call's `since` floor in epoch ms (`ImapSearchCriteria.since` is * `string | Date`), or -1 when the search was unbounded. */ function sinceMs(call: SearchCall): number { @@ -2284,3 +2320,165 @@ describe("mailSync — calendar thread bundling end-to-end", () => { expect(fetchAttachmentCalls).toHaveLength(1); // read back from the store }); }); + +/** The event both end-to-end fixtures answer. */ +const E2E_UID = "evt-e2e@example.test"; +const E2E_MARKER = `rsvp:${E2E_UID}:series:guest@example.test`; + +/** A `METHOD:REPLY` declining `E2E_UID`. */ +const E2E_DECLINE = [ + "BEGIN:VCALENDAR", + "METHOD:REPLY", + "BEGIN:VEVENT", + `UID:${E2E_UID}`, + "ATTENDEE;CN=Sam Guest;PARTSTAT=DECLINED:mailto:guest@example.test", + "END:VEVENT", + "END:VCALENDAR", +].join("\r\n"); + +/** + * Where in `callLog` the `setMany` invocation whose keys satisfy `match` + * happened, or -1. + * + * A pass makes TWO `setMany` calls — the fold markers, flushed before + * `saveLinks`, and the per-root `ThreadMeta`, written deliberately after it — + * and `callLog` records only the method name. So an ordering assertion has to + * identify WHICH write it means by the keys that write carried; a bare + * `indexOf`/`lastIndexOf("setMany")` silently asserts about the other one. + * The nth `"setMany"` entry in `callLog` is `setManyCalls[n]`. + */ +function setManyPosition( + built: { callLog: string[]; setManyCalls: string[][] }, + match: (keys: string[]) => boolean +): number { + const nth = built.setManyCalls.findIndex(match); + if (nth < 0) return -1; + let seen = -1; + for (let i = 0; i < built.callLog.length; i++) { + if (built.callLog[i] !== "setMany") continue; + seen++; + if (seen === nth) return i; + } + return -1; +} + +/** + * One `mailSync` pass over a thread carrying a DECLINED response AND ordinary + * correspondence. + * + * The ordinary message is load-bearing, not scenery: a thread of nothing but + * folded responses emits no link at all, so `saveLinks` is never called and + * there is no write for the marker flush to be ordered against. + */ +async function declinePass(opts: { failSaveLinks?: boolean } = {}) { + const built = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [ + box("INBOX", [ + calendarMessage({ + uid: 51, + messageId: "", + root: "", + }), + plainMessage({ uid: 52, root: "" }), + ]), + ], + attachments: { [buildAttachmentRef("INBOX", 51, "2")]: icsBytes(E2E_DECLINE) }, + ...(opts.failSaveLinks ? { failSaveLinks: true } : {}), + }); + + // A throwing `saveLinks` must not abort the test — the assertion is about + // what survived, not that the pass succeeded. + await mailSync(built.host, [INBOX_CHANNEL], RECENT_ISO).catch(() => {}); + return built; +} + +describe("mailSync — attendee responses end-to-end", () => { + it("folds a decline that arrives on a LATER pass than the invite it threads onto", async () => { + // The case a single-pass fixture cannot reach. Pass 1 ingests the + // invitation, classifies its root as "no bundle" and records that + // decision; pass 2's response threads onto that same root, so it is only + // seen at all if the cached root keeps being examined for new messages. + const invite = { + ...calendarMessage({ + uid: 50, + messageId: "", + root: "", + date: daysAgo(3), + }), + subject: "Invitation: Weekly sync", + } as ImapMessage; + const reply = calendarMessage({ + uid: 51, + messageId: "", + root: "", + date: daysAgo(1), + }); + + const built = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [invite])], + attachments: { + [buildAttachmentRef("INBOX", 50, "2")]: icsBytes( + ics({ method: "REQUEST", uid: E2E_UID, sequence: 0 }) + ), + }, + }); + + await mailSync(built.host, [INBOX_CHANNEL], RECENT_ISO); + expect(built.savedNotes).toHaveLength(0); + expect(linkFor(built.savedLinks, "icloud-mail:thread:invite@example.test").title).toBe( + "Invitation: Weekly sync" + ); + + // Pass 2: the response arrives, threading onto the invitation's Message-ID. + addMessage(built.mailboxes.get("INBOX")!, reply); + built.attachments[buildAttachmentRef("INBOX", 51, "2")] = icsBytes(E2E_DECLINE); + built.savedLinks.length = 0; + + await mailSync(built.host, [INBOX_CHANNEL], RECENT_ISO); + + expect(built.savedNotes).toHaveLength(1); + expect(built.savedNotes[0]).toMatchObject({ + thread: { source: `icaluid:${E2E_UID}` }, + key: "reply-1@example.test", + content: "Sam Guest declined.", + }); + // …and the response is not ALSO left in the mail thread, which still + // carries only the invitation's own note. + expect(noteKeys(linkFor(built.savedLinks, "icloud-mail:thread:invite@example.test"))).toEqual([ + "invite@example.test", + ]); + }); + + it("writes every fold marker BEFORE saveLinks", async () => { + const built = await declinePass(); + + const firstSaveLinks = built.callLog.indexOf("saveLinks"); + expect(firstSaveLinks).toBeGreaterThan(-1); + + const markerWrite = setManyPosition(built, (keys) => keys.some((k) => k.startsWith("rsvp:"))); + expect(markerWrite).toBeGreaterThan(-1); + expect(markerWrite).toBeLessThan(firstSaveLinks); + + // The other write goes the other way ON PURPOSE (see `mailSync`): thread + // metadata is persisted AFTER the save so a throw re-runs the initial-sync + // discipline. Asserted here so "make both writes early" is not mistaken + // for a fix if the marker ordering ever regresses. + const metaWrite = setManyPosition(built, (keys) => keys.some((k) => k.startsWith("thread:"))); + expect(metaWrite).toBeGreaterThan(firstSaveLinks); + }); + + it("keeps the marker written when saveLinks throws, so the note is not re-emitted", async () => { + const built = await declinePass({ failSaveLinks: true }); + + expect(built.callLog).toContain("saveLinks"); // the pass really got that far + expect(await built.host.get(E2E_MARKER)).toBe("DECLINED"); + + // The point of writing the marker first: this pass left a note on the + // event thread and saved nothing else, so a re-run must not write that + // note a second time and drag a thread people had read back to unread. + await mailSync(built.host, [INBOX_CHANNEL], RECENT_ISO).catch(() => {}); + expect(built.savedNotes).toHaveLength(1); + }); +}); From a867aa4a3444ab7429109a508732c35960643dd9 Mon Sep 17 00:00:00 2001 From: Kris Braun Date: Mon, 3 Aug 2026 17:33:00 -0400 Subject: [PATCH 08/10] fix(apple): stop a folded response raising unread on the thread it left MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folding an attendee response removes its note from the mail thread, but the thread's read state still counted that message as newly-arrived unseen mail. A conversation whose only new message was a response therefore surfaced as unread with nothing new in it — before the fold, that unread at least arrived with the response's own note attached. Worse on a root that bundles onto its event (an updated invitation or a cancellation, carrying an `icaluid:` alias): there the mail link IS the event's thread, so a bare acceptance dragged the event thread back to unread through the link save — the very thing folding avoids on the note side, arriving by the one route folding does not cover. Raising unread now considers only messages that still carry a note here. Clearing unread is unchanged and still considers every message: an unseen response is unseen mail, so a pass that sees one makes no claim about read state in either direction. Also documents that a response the fold does not recognise (no ATTENDEE line, or a participation status outside accepted/declined/tentative) does still reach the calendar-part classifier, and tidies the fixtures that hand-rolled ICS bodies the test helpers already build. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GcnBUEdW86ovpv1pT2d3b3 --- connectors/apple/src/mail/calendar-bundle.ts | 26 ++++--- connectors/apple/src/mail/sync.test.ts | 34 +++------- connectors/apple/src/mail/transform.test.ts | 71 ++++++++++++++++++++ connectors/apple/src/mail/transform.ts | 12 +++- 4 files changed, 105 insertions(+), 38 deletions(-) diff --git a/connectors/apple/src/mail/calendar-bundle.ts b/connectors/apple/src/mail/calendar-bundle.ts index c54e7c4b..5e3f1e11 100644 --- a/connectors/apple/src/mail/calendar-bundle.ts +++ b/connectors/apple/src/mail/calendar-bundle.ts @@ -13,9 +13,12 @@ * the response is attached to the event's thread as a note of its own — or, if * it is a bare acceptance saying nothing the guest list does not already show, * dropped entirely — and its message is kept out of the mail thread either - * way. A response never reaches `classifyICS` in a sync pass, so the - * non-bundling verdict this file gives one says nothing about what becomes of - * the message. + * way. A response the fold RECOGNISES never reaches `classifyICS` in a sync + * pass, so the non-bundling verdict this file gives one says nothing about what + * becomes of the message. A `METHOD:REPLY` the fold does not recognise — no + * `ATTENDEE` line, or a `PARTSTAT` outside accepted/declined/tentative such as + * `NEEDS-ACTION` or `DELEGATED` — does fall through to here, and is classified + * as non-bundling like any other part. * * Ports the Google connector's `classifyCalendarThread` decision * (`google/src/mail/gmail-api.ts`) — the product-approved rule for which ICS @@ -55,18 +58,19 @@ export function isCalendarAttachment(mimeType: string): boolean { * Classify one ICS (VCALENDAR) text's relationship to its event, per the * product-approved rule (see module doc): * - * | ICS content | Action | - * |--------------------------------------|--------------------------------------------| - * | `METHOD:CANCEL` | bundle | - * | `METHOD:REQUEST` with `SEQUENCE > 0` | bundle | - * | `METHOD:REQUEST` with `SEQUENCE == 0`| skip | - * | `METHOD:REPLY` (an RSVP) | folded onto the event thread (see sync.ts) | + * | ICS content | Action | + * |---------------------------------------|--------------------------------------------| + * | `METHOD:CANCEL` | bundle | + * | `METHOD:REQUEST` with `SEQUENCE > 0` | bundle | + * | `METHOD:REQUEST` with `SEQUENCE == 0` | skip | + * | `METHOD:REPLY` (an RSVP) | folded onto the event thread (see sync.ts) | * * Returns `null` for everything that does not bundle (including no parseable * UID at all) so callers can uniformly treat every non-bundling case — RSVP, * bare invite, or unparseable text — the same way. A `METHOD:REPLY` still - * returns `null` here, but in a sync pass it is folded before this function is - * ever offered the part, so that `null` is only reachable from another caller. + * returns `null` here; in a sync pass it is normally folded before this + * function is offered the part, so that `null` is reached only by another + * caller or by a response the fold does not recognise (see the module doc). */ export function classifyICS(ics: string): ClassifiedICS | null { const uid = icsProp(ics, "UID"); diff --git a/connectors/apple/src/mail/sync.test.ts b/connectors/apple/src/mail/sync.test.ts index 8b74ef21..92547be8 100644 --- a/connectors/apple/src/mail/sync.test.ts +++ b/connectors/apple/src/mail/sync.test.ts @@ -1999,12 +1999,14 @@ function replyIcs(opts: { partstat: "ACCEPTED" | "DECLINED" | "TENTATIVE"; comment?: string; recurrenceId?: string; + /** The event being answered; defaults to the fold fixtures' own event. */ + uid?: string; }): string { return [ "BEGIN:VCALENDAR", "METHOD:REPLY", "BEGIN:VEVENT", - `UID:${REPLY_UID}`, + `UID:${opts.uid ?? REPLY_UID}`, ...(opts.recurrenceId ? [`RECURRENCE-ID:${opts.recurrenceId}`] : []), `ATTENDEE;CN=Sam Guest;PARTSTAT=${opts.partstat}:mailto:guest@example.test`, ...(opts.comment ? [`COMMENT:${opts.comment}`] : []), @@ -2326,15 +2328,7 @@ const E2E_UID = "evt-e2e@example.test"; const E2E_MARKER = `rsvp:${E2E_UID}:series:guest@example.test`; /** A `METHOD:REPLY` declining `E2E_UID`. */ -const E2E_DECLINE = [ - "BEGIN:VCALENDAR", - "METHOD:REPLY", - "BEGIN:VEVENT", - `UID:${E2E_UID}`, - "ATTENDEE;CN=Sam Guest;PARTSTAT=DECLINED:mailto:guest@example.test", - "END:VEVENT", - "END:VCALENDAR", -].join("\r\n"); +const E2E_DECLINE = replyIcs({ partstat: "DECLINED", uid: E2E_UID }); /** * Where in `callLog` the `setMany` invocation whose keys satisfy `match` @@ -2374,14 +2368,7 @@ async function declinePass(opts: { failSaveLinks?: boolean } = {}) { const built = buildFakeHost({ appleId: "owner@example.test", mailboxes: [ - box("INBOX", [ - calendarMessage({ - uid: 51, - messageId: "", - root: "", - }), - plainMessage({ uid: 52, root: "" }), - ]), + box("INBOX", [replyMessage(), plainMessage({ uid: 52, root: "" })]), ], attachments: { [buildAttachmentRef("INBOX", 51, "2")]: icsBytes(E2E_DECLINE) }, ...(opts.failSaveLinks ? { failSaveLinks: true } : {}), @@ -2408,13 +2395,6 @@ describe("mailSync — attendee responses end-to-end", () => { }), subject: "Invitation: Weekly sync", } as ImapMessage; - const reply = calendarMessage({ - uid: 51, - messageId: "", - root: "", - date: daysAgo(1), - }); - const built = buildFakeHost({ appleId: "owner@example.test", mailboxes: [box("INBOX", [invite])], @@ -2432,8 +2412,10 @@ describe("mailSync — attendee responses end-to-end", () => { ); // Pass 2: the response arrives, threading onto the invitation's Message-ID. - addMessage(built.mailboxes.get("INBOX")!, reply); + addMessage(built.mailboxes.get("INBOX")!, replyMessage()); built.attachments[buildAttachmentRef("INBOX", 51, "2")] = icsBytes(E2E_DECLINE); + // `savedLinks` accumulates across passes and `linkFor` asserts exactly one + // match, so pass 1's link has to be cleared before pass 2's is inspected. built.savedLinks.length = 0; await mailSync(built.host, [INBOX_CHANNEL], RECENT_ISO); diff --git a/connectors/apple/src/mail/transform.test.ts b/connectors/apple/src/mail/transform.test.ts index 81fc793d..a6ccbee6 100644 --- a/connectors/apple/src/mail/transform.test.ts +++ b/connectors/apple/src/mail/transform.test.ts @@ -1069,6 +1069,77 @@ describe("transformMessages — folded attendee responses", () => { ).toContain("guest@example.test"); }); + it("does not raise unread for a newly-arrived folded response, with nothing new left to read", () => { + // Before the fold existed, this thread's unread arrived together with the + // response's own note. The note is now on the event's thread instead, so + // raising unread here would surface a thread with nothing new in it. + const real = msg({ + uid: 1, + messageId: "", + references: [ROOT], + flags: ["\\Seen"], + date: new Date("2026-07-15T09:00:00Z"), + }); + const rsvp = msg({ + uid: 2, + messageId: "", + references: [ROOT], + flags: [], // unseen, and new this pass + date: new Date("2026-07-15T10:00:00Z"), + }); + + const links = transformMessages( + [real, rsvp], + incrementalCtxFor([real, rsvp], { + foldedNoteKeys: new Set(["rsvp@example.test"]), + newMessages: new Set([messageKey(rsvp)]), + }) + ); + + expect(links).toHaveLength(1); + expect(links[0].notes!.map((n) => (n as { key: string }).key)).toEqual(["real@example.test"]); + // Not `unread: false` either — an unseen response is still unseen mail, so + // this pass makes no claim about read state in either direction. + expect("unread" in links[0]).toBe(false); + }); + + it("does not raise unread on a BUNDLED root, where the link IS the event's thread", () => { + // The route the fold does not cover. A root classified as an update or a + // cancellation carries `sources: ["icaluid:…"]`, so this link and the + // calendar event are one thread. A bare acceptance folded away here would + // still drag that event thread back to unread — through `saveLinks` + // rather than `saveNote`, but with the same result for the organiser. + const update = msg({ + uid: 1, + messageId: "", + subject: "Updated invitation: Weekly sync", + flags: ["\\Seen"], + date: new Date("2026-07-15T09:00:00Z"), + }); + const acceptance = msg({ + uid: 2, + messageId: "", + references: [""], + flags: [], // unseen, and new this pass + date: new Date("2026-07-15T10:00:00Z"), + }); + + const links = transformMessages( + [update, acceptance], + incrementalCtxFor([update, acceptance], { + foldedNoteKeys: new Set(["rsvp@example.test"]), + newMessages: new Set([messageKey(acceptance)]), + calendarBundles: new Map([ + ["update@example.test", { uid: "evt-1", kind: "update" as const, eventKnown: true }], + ]), + }) + ); + + expect(links).toHaveLength(1); + expect(links[0].sources).toEqual(["icaluid:evt-1"]); + expect("unread" in links[0]).toBe(false); + }); + it("is unaffected when nothing was folded", () => { const real = msg({ uid: 1, messageId: "", references: [ROOT] }); const links = transform([real]); diff --git a/connectors/apple/src/mail/transform.ts b/connectors/apple/src/mail/transform.ts index 228bcd8c..160e0d7c 100644 --- a/connectors/apple/src/mail/transform.ts +++ b/connectors/apple/src/mail/transform.ts @@ -452,8 +452,18 @@ export function transformMessages( // would let an old unseen message in one folder inherit the "new" status // of an unrelated message that happens to share its uid in another — // re-marking the thread unread on every single poll. + // + // `hasNewUnseen` reads `surviving`, not `msgs`: raising unread is a claim + // that there is something new to read HERE, and a folded response left no + // note on this link. On a bundled root the link IS the event's thread + // (`sources: ["icaluid:…"]`), so counting a folded response here would + // drag the event thread back to unread through `saveLinks` — exactly what + // the fold avoids on the `saveNote` side, arriving by the one route the + // fold does not cover. `allSeen` still reads `msgs`: clearing unread is a + // claim that nothing in the mailbox is unread, and an unseen response is + // still unseen mail, so it correctly holds that claim back. const allSeen = msgs.every((m) => isSeen(m)); - const hasNewUnseen = msgs.some( + const hasNewUnseen = surviving.some( (m) => !isSeen(m) && ctx.newMessages.has(messageKey(m)) ); const incrementalRead: { unread?: boolean } = allSeen From f43ac840fc6b2463af2fcf37c6af09cd7b88563f Mon Sep 17 00:00:00 2001 From: Kris Braun Date: Mon, 3 Aug 2026 18:01:10 -0400 Subject: [PATCH 09/10] fix(apple): keep an attendee response folded on every later sync pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A response folded onto its event's thread was dropped from the mail thread only on the pass that read its calendar part. Because an already-examined part is deliberately never fetched again, every later pass within the rescan window reported nothing folded and put the response's note straight back into the mail thread. On a thread bundled onto its calendar event, that mail thread IS the event's thread — so the raw response email was written onto the organiser's event thread as an ordinary note, which marks the thread unread for everyone but its author. On a mixed conversation the response also came back as a duplicate of the note already on the event thread, and took over the mail thread's title and author again. Foldedness is now durable per thread root (`ThreadMeta.foldedIcs`) and is seeded before any of the pass's short-circuits, so a root whose bundling decision is cached and whose messages have all been examined still reports what it folded. It is retained in lockstep with the examined-part list rather than under a cap of its own: a key can only be dropped alongside the entry that keeps its message off the wire, so the next pass re-reads and re-folds the message instead of resurfacing its note. Also flush the fold markers in a `finally`. Batching them into one write made them all-or-nothing: a note write that failed part-way through a pass took the markers of every response already written with it, and the next pass re-sent exactly those notes and raised unread again. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GcnBUEdW86ovpv1pT2d3b3 --- connectors/apple/src/mail/sync.test.ts | 330 +++++++++++++++++++- connectors/apple/src/mail/sync.ts | 409 ++++++++++++++----------- 2 files changed, 562 insertions(+), 177 deletions(-) diff --git a/connectors/apple/src/mail/sync.test.ts b/connectors/apple/src/mail/sync.test.ts index 92547be8..24168f3a 100644 --- a/connectors/apple/src/mail/sync.test.ts +++ b/connectors/apple/src/mail/sync.test.ts @@ -74,6 +74,13 @@ function buildFakeHost(opts: { * which is what the fold-marker ordering exists to control. */ failSaveLinks?: boolean; + /** + * Make the Nth `saveNote` call (1-based) record itself and then throw, as a + * platform write that fails part-way through a pass would. Everything the + * pass wrote before it — including the fold markers of responses already + * saved — has to survive that throw, or the next pass re-emits those notes. + */ + failSaveNoteOnCall?: number; }) { const stored = new Map(); const savedLinks: NewLinkWithNotes[] = []; @@ -185,6 +192,7 @@ function buildFakeHost(opts: { saveNote: async (note: Record): Promise => { callLog.push("saveNote"); savedNotes.push(note); + if (opts.failSaveNoteOnCall === savedNotes.length) throw new Error("saveNote failed"); return "note-id"; }, setThreadToDo, @@ -1990,6 +1998,139 @@ describe("detectCalendarBundles", () => { eventKnown: true, }); }); + + it("never re-decides a root once recorded, even when a CANCEL arrives later", async () => { + // The other half of the gate split, and the one a message with no calendar + // part cannot reach: this root's decision is cached as "does not bundle", + // and a NEW calendar-bearing message on it is still fetched and examined. + // What must NOT happen is the classification running again — a root that + // flips from "no bundle" to "cancel" changes `sources`' sorted-minimum + // primary source, and `upsert_link` writes a SECOND link row because the + // old primary source is still on file. + const later = msg({ + uid: 67, + messageId: "", + references: [""], + attachments: [CALENDAR_PART], + }); + const { host, stored, fetchAttachmentCalls } = bundleHost({ + attachments: { + [buildAttachmentRef("INBOX", 67, "2")]: icsBytes( + ics({ method: "CANCEL", uid: "evt-settled" }) + ), + }, + }); + const meta = new Map([ + ["settled@example.test", { channelId: "mail:INBOX", bundle: { classified: null } }], + ]); + + const { bundles } = await detectCalendarBundles( + host, + "session-1", + [{ ...later, mailbox: "INBOX" }], + meta, + new Set() + ); + + // Examined (that is the other gate), but the recorded decision stands. + expect(fetchAttachmentCalls).toHaveLength(1); + expect(bundles.has("settled@example.test")).toBe(false); + expect(meta.get("settled@example.test")!.bundle).toEqual({ classified: null }); + expect(stored.get("cancel-email:evt-settled")).toBeUndefined(); + }); + + it("does not let an attendee response settle the bundling question for its root", async () => { + // A response answers a question it was never asked. Classifying it would + // record the root as "evaluated, does not bundle" — permanently — so the + // rescheduling notice that arrives on the same root afterwards could never + // bundle the mail thread onto the event. + const reply = calendarMessage({ + uid: 51, + messageId: "", + root: "", + }); + const update = msg({ + uid: 52, + messageId: "", + references: [""], + attachments: [CALENDAR_PART], + }); + const { host } = bundleHost({ + attachments: { + [buildAttachmentRef("INBOX", 51, "2")]: icsBytes( + replyIcs({ partstat: "DECLINED", uid: "evt-reply-only" }) + ), + [buildAttachmentRef("INBOX", 52, "2")]: icsBytes( + ics({ method: "REQUEST", uid: "evt-reply-only", sequence: 2 }) + ), + }, + }); + const meta = metaFor(["invite@example.test"]); + + // Pass 1: the response alone. The root must be left UNDECIDED. + await detectCalendarBundles( + host, + "session-1", + [{ ...reply, mailbox: "INBOX" }], + meta, + new Set() + ); + expect(meta.get("invite@example.test")!.bundle).toBeUndefined(); + + // Pass 2: the rescheduling notice threads onto the same root and still + // gets its say. + const { bundles } = await detectCalendarBundles( + host, + "session-1", + [ + { ...reply, mailbox: "INBOX" }, + { ...update, mailbox: "INBOX" }, + ], + meta, + new Set() + ); + expect(bundles.get("invite@example.test")).toEqual({ + uid: "evt-reply-only", + kind: "update", + eventKnown: false, + }); + }); + + it("caps the examined-part list so one thread cannot grow the document forever", async () => { + // A full SEEN_ICS_MAX (200) of already-examined parts, then one more. The + // oldest is dropped rather than the array growing without bound — this + // document is rewritten on every pass. + const priorKeys = Array.from({ length: 200 }, (_, i) => `seen-${i}@example.test`); + const newest = msg({ + uid: 68, + messageId: "", + references: [""], + attachments: [CALENDAR_PART], + }); + const { host } = bundleHost({ + attachments: { + [buildAttachmentRef("INBOX", 68, "2")]: icsBytes( + ics({ method: "REQUEST", uid: "evt-capped", sequence: 0 }) + ), + }, + }); + const meta = new Map([ + ["capped@example.test", { channelId: "mail:INBOX", seenIcs: priorKeys }], + ]); + + await detectCalendarBundles( + host, + "session-1", + [{ ...newest, mailbox: "INBOX" }], + meta, + new Set() + ); + + const seen = meta.get("capped@example.test")!.seenIcs!; + expect(seen).toHaveLength(200); + expect(seen).not.toContain("seen-0@example.test"); + expect(seen[seen.length - 1]).toBe("seen-newest@example.test"); + }); }); const REPLY_UID = "evt-fold@example.test"; @@ -1999,16 +2140,20 @@ function replyIcs(opts: { partstat: "ACCEPTED" | "DECLINED" | "TENTATIVE"; comment?: string; recurrenceId?: string; - /** The event being answered; defaults to the fold fixtures' own event. */ - uid?: string; + /** The event being answered; defaults to the fold fixtures' own event. + * `null` omits the UID line entirely (a malformed response). */ + uid?: string | null; + /** The responder. Defaults to the fold fixtures' own guest. */ + attendee?: { name: string; email: string }; }): string { + const attendee = opts.attendee ?? { name: "Sam Guest", email: "guest@example.test" }; return [ "BEGIN:VCALENDAR", "METHOD:REPLY", "BEGIN:VEVENT", - `UID:${opts.uid ?? REPLY_UID}`, + ...(opts.uid === null ? [] : [`UID:${opts.uid ?? REPLY_UID}`]), ...(opts.recurrenceId ? [`RECURRENCE-ID:${opts.recurrenceId}`] : []), - `ATTENDEE;CN=Sam Guest;PARTSTAT=${opts.partstat}:mailto:guest@example.test`, + `ATTENDEE;CN=${attendee.name};PARTSTAT=${opts.partstat}:mailto:${attendee.email}`, ...(opts.comment ? [`COMMENT:${opts.comment}`] : []), "END:VEVENT", "END:VCALENDAR", @@ -2217,6 +2362,122 @@ describe("detectCalendarBundles — attendee responses", () => { expect(built.fetchAttachmentCalls).toHaveLength(1); expect([...foldedNoteKeys]).toEqual(["reply-1@example.test"]); }); + + it("records what it folded on the root's metadata", async () => { + const built = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [replyMessage()])], + attachments: { + [buildAttachmentRef("INBOX", 51, "2")]: icsBytes(replyIcs({ partstat: "DECLINED" })), + }, + }); + const meta = new Map([["invite@example.test", { channelId: "INBOX" }]]); + const changed = new Set(); + + await detectCalendarBundles( + built.host, + "session-1", + [{ ...replyMessage(), mailbox: "INBOX" }], + meta, + changed + ); + + expect(meta.get("invite@example.test")!.foldedIcs).toEqual(["reply-1@example.test"]); + expect(changed.has("invite@example.test")).toBe(true); + }); + + it("keeps reporting a response folded on an earlier pass, without re-reading it", async () => { + // The fold has to outlive the fetch that discovered it. `seenIcs` keeps an + // already-examined response off IMAP, so nothing in a later pass re-parses + // it — if foldedness were derived from this pass's fetches, the response + // would silently return to the mail thread on the very next poll. + const built = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [replyMessage()])], + attachments: {}, // a fetch would throw on lookup miss — that IS the assertion + }); + const meta = new Map([ + [ + "invite@example.test", + { + channelId: "INBOX", + bundle: { classified: null }, + seenIcs: ["reply-1@example.test"], + foldedIcs: ["reply-1@example.test"], + }, + ], + ]); + + const { foldedNoteKeys } = await detectCalendarBundles( + built.host, + "session-1", + [{ ...replyMessage(), mailbox: "INBOX" }], + meta, + new Set() + ); + + expect([...foldedNoteKeys]).toEqual(["reply-1@example.test"]); + expect(built.fetchAttachmentCalls).toHaveLength(0); + expect(built.savedNotes).toHaveLength(0); + }); + + it("leaves a response carrying no event id as ordinary mail", async () => { + // No UID means no `icaluid:` thread to address and no way to scope the + // dedup marker, so there is nowhere to fold it TO. Better an ordinary mail + // note than a note attached to nothing. + const { host, savedNotes, foldedOf } = await runFold( + replyIcs({ partstat: "DECLINED", uid: null }) + ); + + expect(savedNotes).toHaveLength(0); + expect(foldedOf).toHaveLength(0); + }); + + it("keeps the markers of responses already written when a later saveNote throws", async () => { + // Batching the markers into one write must not make them all-or-nothing: + // the first response is on its event thread for good, so losing its marker + // would have the next pass write that note again and re-raise unread on a + // thread people had read. + const first = calendarMessage({ + uid: 51, + messageId: "", + root: "", + }); + const second = calendarMessage({ + uid: 52, + messageId: "", + root: "", + }); + const built = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [first, second])], + attachments: { + [buildAttachmentRef("INBOX", 51, "2")]: icsBytes(replyIcs({ partstat: "DECLINED" })), + [buildAttachmentRef("INBOX", 52, "2")]: icsBytes( + replyIcs({ + partstat: "DECLINED", + attendee: { name: "Robin Guest", email: "robin@example.test" }, + }) + ), + }, + failSaveNoteOnCall: 2, + }); + + await expect( + detectCalendarBundles( + built.host, + "session-1", + [ + { ...first, mailbox: "INBOX" }, + { ...second, mailbox: "INBOX" }, + ], + new Map([["invite@example.test", { channelId: "INBOX" }]]), + new Set() + ) + ).rejects.toThrow("saveNote failed"); + + expect(await built.host.get(`rsvp:${REPLY_UID}:series:guest@example.test`)).toBe("DECLINED"); + }); }); describe("mailSync — calendar thread bundling end-to-end", () => { @@ -2463,4 +2724,65 @@ describe("mailSync — attendee responses end-to-end", () => { await mailSync(built.host, [INBOX_CHANNEL], RECENT_ISO).catch(() => {}); expect(built.savedNotes).toHaveLength(1); }); + + it("keeps a folded response out of a mixed mail thread on EVERY later pass", async () => { + // A response stays inside the 30-day rescan window for a month, so the + // thread carrying it is rebuilt on every poll. The fold has to survive all + // of them — and it cannot be re-derived from the fetch, because an + // already-examined part is deliberately never fetched again. + const built = await declinePass(); + expect( + noteKeys(linkFor(built.savedLinks, "icloud-mail:thread:invite@example.test")) + ).toEqual(["plain-52@example.test"]); + expect(built.savedNotes).toHaveLength(1); + + built.savedLinks.length = 0; + await mailSync(built.host, [INBOX_CHANNEL], RECENT_ISO); + + const link = linkFor(built.savedLinks, "icloud-mail:thread:invite@example.test"); + expect(noteKeys(link)).toEqual(["plain-52@example.test"]); + // …and the thread is still described by the ordinary message, not + // re-titled and re-attributed to the responder. + expect(link.title).toBe("Re: Weekly sync"); + // No second copy of the response on the event thread either. + expect(built.savedNotes).toHaveLength(1); + }); + + it("keeps a folded response off a BUNDLED root's thread on every later pass", async () => { + // The worst case. A bundled root's Plot thread IS the calendar event's + // thread, so a response that leaked back into the mail thread would be + // written straight onto the organiser's event thread as a raw email — and + // a note is exactly what marks that thread unread for everyone but its + // author, which is the outcome the fold exists to prevent. + const cancellation = { + ...calendarMessage({ + uid: 50, + messageId: "", + root: "", + date: daysAgo(3), + }), + subject: "Cancelled: Weekly sync", + } as ImapMessage; + const built = buildFakeHost({ + appleId: "owner@example.test", + mailboxes: [box("INBOX", [cancellation, replyMessage()])], + attachments: { + [buildAttachmentRef("INBOX", 50, "2")]: icsBytes(ics({ method: "CANCEL", uid: E2E_UID })), + [buildAttachmentRef("INBOX", 51, "2")]: icsBytes(E2E_DECLINE), + }, + }); + + await mailSync(built.host, [INBOX_CHANNEL], RECENT_ISO); + const first = linkFor(built.savedLinks, "icloud-mail:thread:invite@example.test"); + expect(first.sources).toEqual([`icaluid:${E2E_UID}`]); + expect(noteKeys(first)).toEqual(["invite@example.test"]); + + built.savedLinks.length = 0; + await mailSync(built.host, [INBOX_CHANNEL], RECENT_ISO); + + const second = linkFor(built.savedLinks, "icloud-mail:thread:invite@example.test"); + expect(second.sources).toEqual([`icaluid:${E2E_UID}`]); + expect(noteKeys(second)).toEqual(["invite@example.test"]); + expect(built.savedNotes).toHaveLength(1); + }); }); diff --git a/connectors/apple/src/mail/sync.ts b/connectors/apple/src/mail/sync.ts index 186353ce..fce985a9 100644 --- a/connectors/apple/src/mail/sync.ts +++ b/connectors/apple/src/mail/sync.ts @@ -100,6 +100,32 @@ export type ThreadMeta = { * pass and an unbounded array is not worth the tail risk. */ seenIcs?: string[]; + /** + * Note keys (see `noteKeyOf`) of messages already folded onto a calendar + * event's thread — attendee responses that belong on the event rather than + * in a mail thread of their own (see `detectCalendarBundles`). + * + * Durable, and deliberately NOT re-derived from what this pass fetched. + * `seenIcs` keeps an already-examined response off IMAP, so a later pass + * never re-parses it and has no way to rediscover that it was folded: it + * would put the response's note back into the mail thread, and on a BUNDLED + * root that mail thread is the event's own thread, so the raw response email + * would land there and mark it unread for everyone but its author — the + * exact outcome the fold exists to prevent. (The other mail connectors + * re-derive their folded set on every pass because they re-read the message + * on every pass; this field is the price of skipping that fetch.) + * + * Retained in lockstep with `seenIcs` instead of under a cap of its own. + * Every folded key is also a seen key, so filtering against what `seenIcs` + * kept bounds this array by SEEN_ICS_MAX while making eviction harmless: a + * key can only be dropped together with its `seenIcs` entry, which puts the + * message back among the unexamined and has the next pass re-fetch and + * re-fold it. (The response's own `rsvp:` marker is a separate, uncapped + * key, so `alreadyFolded` still suppresses a second note.) An independent + * cap would have no such backstop — it would drop a folded key while the + * message stayed un-fetched, and the note would resurface permanently. + */ + foldedIcs?: string[]; }; /** Cap on `ThreadMeta.seenIcs`; see its doc. */ @@ -300,7 +326,11 @@ export async function reconcileTodoFlags( * organiser's event thread back to unread. * * Returns the per-root `bundles` map plus `foldedNoteKeys`: note keys of - * messages this function consumed itself. + * messages this function consumed itself. That set is NOT limited to what this + * pass fetched — it is seeded for every root from the durable + * `ThreadMeta.foldedIcs`, because a response examined on an earlier pass is + * deliberately never re-fetched and so could never be recognised again. See + * `ThreadMeta.foldedIcs`. * * The caller owns the store I/O: `meta` arrives pre-loaded (one read per root * for the pass, shared with home-channel resolution) and is MUTATED in place @@ -329,7 +359,9 @@ export async function detectCalendarBundles( ): Promise<{ bundles: Map; foldedNoteKeys: Set }> { const bundles = new Map(); const foldedNoteKeys = new Set(); - /** Fold markers to persist, flushed in ONE `setMany` before returning. */ + /** Fold markers to persist, flushed in ONE `setMany` in the `finally` + * below — so a `saveNote` that throws mid-pass cannot take the markers of + * responses already written with it. */ const rsvpMarkers: [string, string][] = []; let knownUids: Set | null = null; const resolveEventKnown = async (uid: string): Promise => { @@ -337,194 +369,220 @@ export async function detectCalendarBundles( return knownUids.has(uid); }; - for (const [root, msgs] of groupByRoot(messages).entries()) { - const entry = meta.get(root); - - // The cached BUNDLE decision is served unchanged — see the CACHING doc: - // a classification that flips changes `sources`' sorted-minimum primary - // source and makes `upsert_link` create a second link row. It is consulted - // before anything else, so a root whose ICS-bearing message has aged out - // of the window keeps its decision even when this pass's messages carry - // no calendar part at all. - // - // What is deliberately NOT skipped is the per-message scan below. A root - // is the first `References` entry, and calendar systems thread response - // notifications onto the invite's Message-ID — so the root carrying an - // RSVP is usually the invite's root, already cached as "no bundle" from - // the pass that ingested the invite. Returning early here (as this code - // used to) means an RSVP is never looked at. - const persisted = entry?.bundle; - if (persisted) { - if (persisted.classified) { - bundles.set(root, { - ...persisted.classified, - eventKnown: await resolveEventKnown(persisted.classified.uid), - }); + try { + for (const [root, msgs] of groupByRoot(messages).entries()) { + const entry = meta.get(root); + + // Foldedness is DURABLE, and is seeded here BEFORE any short-circuit + // below — including for a root whose bundle decision is already cached and + // whose messages have all been examined, which does no other work at all. + // Reporting nothing folded for such a root would put every response this + // connector has already moved onto the event's thread straight back into + // the mail thread. See `ThreadMeta.foldedIcs`. + const folded = new Set(entry?.foldedIcs ?? []); + for (const key of folded) foldedNoteKeys.add(key); + + // The cached BUNDLE decision is served unchanged — see the CACHING doc: + // a classification that flips changes `sources`' sorted-minimum primary + // source and makes `upsert_link` create a second link row. It is consulted + // before anything else, so a root whose ICS-bearing message has aged out + // of the window keeps its decision even when this pass's messages carry + // no calendar part at all. + // + // What is deliberately NOT skipped is the per-message scan below. A root + // is the first `References` entry, and calendar systems thread response + // notifications onto the invite's Message-ID — so the root carrying an + // RSVP is usually the invite's root, already cached as "no bundle" from + // the pass that ingested the invite. Returning early here (as this code + // used to) means an RSVP is never looked at. + const persisted = entry?.bundle; + if (persisted) { + if (persisted.classified) { + bundles.set(root, { + ...persisted.classified, + eventKnown: await resolveEventKnown(persisted.classified.uid), + }); + } } - } - // Fetch every calendar part not yet examined. The cheap in-memory filter - // keeps threads with no calendar part off IMAP entirely, and `seenIcs` - // keeps a part that has already been read off it on every later pass - // within the rescan window. - const seen = new Set(entry?.seenIcs ?? []); - const icsByKey = new Map(); - const unexamined = msgs.filter( - (m) => - (m.attachments ?? []).some((a) => isCalendarAttachment(a.mimeType)) && - !seen.has(noteKeyOf(m)) - ); + // Fetch every calendar part not yet examined. The cheap in-memory filter + // keeps threads with no calendar part off IMAP entirely, and `seenIcs` + // keeps a part that has already been read off it on every later pass + // within the rescan window. + const seen = new Set(entry?.seenIcs ?? []); + const icsByKey = new Map(); + const unexamined = msgs.filter( + (m) => + (m.attachments ?? []).some((a) => isCalendarAttachment(a.mimeType)) && + !seen.has(noteKeyOf(m)) + ); - for (const m of unexamined) { - // A merged pass can hold two mailbox copies of ONE message (a copy kept - // in a project folder as well as INBOX). They share a note key, and - // `dedupeCopies` only collapses them later, inside `transformMessages` — - // `unexamined` was filtered against a snapshot of `seen`, so both copies - // are in it. The second copy has nothing new to read, and routing it - // again would emit its response note twice: the marker written below is - // flushed once at the end of the pass, so the second copy would still - // read the pre-pass value and look un-folded. - if (seen.has(noteKeyOf(m))) continue; - - const part = (m.attachments ?? []).find((a) => isCalendarAttachment(a.mimeType))!; - await host.imap.selectMailbox(session, m.mailbox); - const bytes = await host.imap.fetchAttachment(session, m.uid, part.partNumber); - const ics = new TextDecoder("utf-8").decode(bytes); - - // An attendee response is routed here, BEFORE the part is offered to the - // bundling classifier below: `classifyICS` returns null for a REPLY, so - // a reply that fell through would have the root recorded as "evaluated, - // does not bundle" on the strength of a message that answers a question - // it was never asked — and that decision is permanent, so a real invite - // arriving on the same root later would never be classified. - const reply = parseIcsReply(ics, { name: m.from?.[0]?.name ?? null }); - if (reply) { - // The event being answered. `parseIcsReply` reads the ATTENDEE line, - // not the event id, so the UID is read from the same body here. - // Without one there is no thread to address and no way to scope the - // marker, so such a response is left as ordinary mail rather than - // folded into nowhere — but it is still never classified, since a - // reply is not an answer to the bundling question. - const replyUid = icsProp(ics, "UID"); - if (replyUid) { - const priorKey = priorRsvpKey(replyUid, reply.attendeeEmail, reply.occurrence); - const stored = await host.get(priorKey); - - // Order is the library's documented contract: `alreadyFolded` - // FIRST, and only when it is false decide whether to emit. - // Re-emitting a note the thread already carries re-applies its - // unread intent and drags the thread back to unread for everyone - // who had read it — and a response inside the 30-day rescan window - // is re-read on every pass. - if ( - !alreadyFolded(stored, reply) && - shouldEmitRsvpNote(reply, isNonAcceptance(stored)) - ) { - // `saveNote` returns null when no thread carries `icaluid:` - // yet (the calendar event has not synced); `deferUntilThread` has - // the platform hold the note and attach it once that thread - // appears. - await host.integrations.saveNote({ - thread: { source: `icaluid:${replyUid}` }, - key: noteKeyOf(m), - content: composeRsvpNote(reply), - contentType: "markdown", - ...(m.date ? { created: m.date } : {}), - author: { - email: reply.attendeeEmail, - ...(reply.attendeeName ? { name: reply.attendeeName } : {}), - }, - // Explicit on both paths. An omitted flag does NOT mean "leave - // read state alone" — attaching a note already marks the thread - // unread for every recipient except its author, so only an - // explicit false overrides it. - unread: !initialRoots.has(root), - deferUntilThread: true, - }); - // Recorded ONLY on the path that emits, and regardless of the - // return value: a deferred note returns no id, and gating on it - // would leave a deferred non-acceptance unrecorded forever, - // wrongly treating a later bare acceptance as reversing nothing. - // The marker holds the last response actually folded onto the - // thread — for every emitted response, acceptances included, - // which is what lets `alreadyFolded` recognise a repeat of ANY - // partstat. - rsvpMarkers.push([priorKey, reply.partstat]); + for (const m of unexamined) { + // A merged pass can hold two mailbox copies of ONE message (a copy kept + // in a project folder as well as INBOX). They share a note key, and + // `dedupeCopies` only collapses them later, inside `transformMessages` — + // `unexamined` was filtered against a snapshot of `seen`, so both copies + // are in it. The second copy has nothing new to read, and routing it + // again would emit its response note twice: the marker written below is + // flushed once at the end of the pass, so the second copy would still + // read the pre-pass value and look un-folded. + if (seen.has(noteKeyOf(m))) continue; + + const part = (m.attachments ?? []).find((a) => isCalendarAttachment(a.mimeType))!; + await host.imap.selectMailbox(session, m.mailbox); + const bytes = await host.imap.fetchAttachment(session, m.uid, part.partNumber); + const ics = new TextDecoder("utf-8").decode(bytes); + + // An attendee response is routed here, BEFORE the part is offered to the + // bundling classifier below: `classifyICS` returns null for a REPLY, so + // a reply that fell through would have the root recorded as "evaluated, + // does not bundle" on the strength of a message that answers a question + // it was never asked — and that decision is permanent, so a real invite + // arriving on the same root later would never be classified. + const reply = parseIcsReply(ics, { name: m.from?.[0]?.name ?? null }); + if (reply) { + // The event being answered. `parseIcsReply` reads the ATTENDEE line, + // not the event id, so the UID is read from the same body here. + // Without one there is no thread to address and no way to scope the + // marker, so such a response is left as ordinary mail rather than + // folded into nowhere — but it is still never classified, since a + // reply is not an answer to the bundling question. + const replyUid = icsProp(ics, "UID"); + if (replyUid) { + const priorKey = priorRsvpKey(replyUid, reply.attendeeEmail, reply.occurrence); + const stored = await host.get(priorKey); + + // Order is the library's documented contract: `alreadyFolded` + // FIRST, and only when it is false decide whether to emit. + // Re-emitting a note the thread already carries re-applies its + // unread intent and drags the thread back to unread for everyone + // who had read it — and a response inside the 30-day rescan window + // is re-read on every pass. + if ( + !alreadyFolded(stored, reply) && + shouldEmitRsvpNote(reply, isNonAcceptance(stored)) + ) { + // `saveNote` returns null when no thread carries `icaluid:` + // yet (the calendar event has not synced); `deferUntilThread` has + // the platform hold the note and attach it once that thread + // appears. + await host.integrations.saveNote({ + thread: { source: `icaluid:${replyUid}` }, + key: noteKeyOf(m), + content: composeRsvpNote(reply), + contentType: "markdown", + ...(m.date ? { created: m.date } : {}), + author: { + email: reply.attendeeEmail, + ...(reply.attendeeName ? { name: reply.attendeeName } : {}), + }, + // Explicit on both paths. An omitted flag does NOT mean "leave + // read state alone" — attaching a note already marks the thread + // unread for every recipient except its author, so only an + // explicit false overrides it. + unread: !initialRoots.has(root), + deferUntilThread: true, + }); + // Recorded ONLY on the path that emits, and regardless of the + // return value: a deferred note returns no id, and gating on it + // would leave a deferred non-acceptance unrecorded forever, + // wrongly treating a later bare acceptance as reversing nothing. + // The marker holds the last response actually folded onto the + // thread — for every emitted response, acceptances included, + // which is what lets `alreadyFolded` recognise a repeat of ANY + // partstat. + rsvpMarkers.push([priorKey, reply.partstat]); + } + + // Folded whether or not a note was written — a bare acceptance is + // dropped from the mail thread rather than left to become an email + // thread of its own. Recorded on the root's metadata as well as + // reported to this caller, so every LATER pass keeps dropping it + // without re-reading the part (see `ThreadMeta.foldedIcs`). + foldedNoteKeys.add(noteKeyOf(m)); + folded.add(noteKeyOf(m)); } - // Folded whether or not a note was written — a bare acceptance is - // dropped from the mail thread rather than left to become an email - // thread of its own. - foldedNoteKeys.add(noteKeyOf(m)); + // Deliberately NOT added to `icsByKey`: see the comment above. + seen.add(noteKeyOf(m)); + continue; } - // Deliberately NOT added to `icsByKey`: see the comment above. + icsByKey.set(noteKeyOf(m), ics); seen.add(noteKeyOf(m)); - continue; } - icsByKey.set(noteKeyOf(m), ics); - seen.add(noteKeyOf(m)); - } - - if (entry && unexamined.length > 0) { - entry.seenIcs = [...seen].slice(-SEEN_ICS_MAX); - changed.add(root); - } + if (entry && unexamined.length > 0) { + const retained = [...seen].slice(-SEEN_ICS_MAX); + entry.seenIcs = retained; + // Folded keys ride `seenIcs`'s retention exactly rather than carrying + // a cap of their own — see `ThreadMeta.foldedIcs`. Every folded key is + // also a seen key, so this bounds the array by SEEN_ICS_MAX while + // guaranteeing a key is only ever dropped alongside the `seenIcs` + // entry that keeps its message off IMAP; the next pass re-fetches and + // re-folds it instead of surfacing its note again. + const kept = new Set(retained); + const keptFolded = [...folded].filter((k) => kept.has(k)); + if (keptFolded.length > 0) entry.foldedIcs = keptFolded; + else delete entry.foldedIcs; + changed.add(root); + } - // Everything past here is the ONE-TIME bundling classification; a root - // that already has a decision keeps it. - if (persisted) continue; - - // Nothing to classify from: either no calendar part at all, or every part - // was read on an earlier pass (in which case a decision was recorded then - // and `persisted` already sent us round). Leave the root undecided so a - // part arriving on a later pass is still evaluated. - if (icsByKey.size === 0) continue; - - let classified: ClassifiedICS | null = null; - for (const m of msgs) { - const ics = icsByKey.get(noteKeyOf(m)); - if (!ics) continue; - classified = classifyICS(ics); - if (!classified) continue; // bare invite or RSVP — check the thread's other messages - - if (classified.kind === "cancel") { - await host.set(`cancel-email:${classified.uid}`, { at: new Date().toISOString() }); + // Everything past here is the ONE-TIME bundling classification; a root + // that already has a decision keeps it. + if (persisted) continue; + + // Nothing to classify from: either no calendar part at all, or every part + // was read on an earlier pass (in which case a decision was recorded then + // and `persisted` already sent us round). Leave the root undecided so a + // part arriving on a later pass is still evaluated. + if (icsByKey.size === 0) continue; + + let classified: ClassifiedICS | null = null; + for (const m of msgs) { + const ics = icsByKey.get(noteKeyOf(m)); + if (!ics) continue; + classified = classifyICS(ics); + if (!classified) continue; // bare invite or RSVP — check the thread's other messages + + if (classified.kind === "cancel") { + await host.set(`cancel-email:${classified.uid}`, { at: new Date().toISOString() }); + } + break; // this thread is classified; stop scanning its remaining messages } - break; // this thread is classified; stop scanning its remaining messages - } - // Record the decision — including explicit "no bundle" — so this root is - // never re-evaluated on a later pass (see the caching doc above). - if (entry) { - entry.bundle = { classified }; - changed.add(root); - } + // Record the decision — including explicit "no bundle" — so this root is + // never re-evaluated on a later pass (see the caching doc above). + if (entry) { + entry.bundle = { classified }; + changed.add(root); + } - if (classified) { - bundles.set(root, { ...classified, eventKnown: await resolveEventKnown(classified.uid) }); + if (classified) { + bundles.set(root, { ...classified, eventKnown: await resolveEventKnown(classified.uid) }); + } } + } finally { + // ONE `setMany`, per `MailHost.setMany`'s contract: a pass can fold many + // responses and a `set` each would burn a request apiece. + // + // Written HERE — before `mailSync`'s `saveLinks` — deliberately, and + // opposite to `ThreadMeta`, which is persisted AFTER `saveLinks` so a + // throw re-runs the initial-sync discipline. The two fail in opposite + // directions: a marker written after a throwing `saveLinks` would leave + // the note on file with nothing recording it, so the next pass would + // re-emit it and re-raise unread on a thread people had already read. Do + // not "tidy" these into one place. + // + // In a `finally` so batching costs nothing in durability: a `saveNote` + // that throws part-way through the pass would otherwise take every marker + // with it, including those for responses already written to their event + // threads, and the next pass would re-emit exactly those notes and + // re-raise unread. The flush still happens once, so the request budget is + // unaffected. The original error propagates from here as normal. + if (rsvpMarkers.length > 0) await host.setMany(rsvpMarkers); } - // ONE `setMany`, per `MailHost.setMany`'s contract: a pass can fold many - // responses and a `set` each would burn a request apiece. - // - // Written HERE — before `mailSync`'s `saveLinks` — deliberately, and - // opposite to `ThreadMeta`, which is persisted AFTER `saveLinks` so a throw - // re-runs the initial-sync discipline. The two fail in opposite directions: - // a marker written after a throwing `saveLinks` would leave the note on file - // with nothing recording it, so the next pass would re-emit it and re-raise - // unread on a thread people had already read. Do not "tidy" these into one - // place. - // - // Batching costs one thing a write per note would not: if a later `saveNote` - // in this pass throws, NO markers are written — including for responses - // already emitted before it, which are then re-emitted on the next pass. - // Accepted for the request budget, and it fails in the same direction as - // everything else here: a repeated note, never a missing one. - if (rsvpMarkers.length > 0) await host.setMany(rsvpMarkers); - return { bundles, foldedNoteKeys }; } @@ -821,6 +879,11 @@ export async function mailSync( // `seenIcs` means every calendar part in the rescan window is // re-fetched on every poll. ...(prev?.seenIcs ? { seenIcs: prev.seenIcs } : {}), + // Likewise — and this one is not merely a cost: a dropped `foldedIcs` + // puts every attendee response already moved onto its event's thread + // back into the mail thread, which on a bundled root IS that event's + // thread. See `ThreadMeta.foldedIcs`. + ...(prev?.foldedIcs ? { foldedIcs: prev.foldedIcs } : {}), }); if (!prev || prev.channelId !== channelId) changedMeta.add(root); } From f475f12a6cda37acea002db16d3c32945e5460d4 Mon Sep 17 00:00:00 2001 From: Kris Braun Date: Mon, 3 Aug 2026 18:01:16 -0400 Subject: [PATCH 10/10] test(apple): file the METHOD:REPLY case under the classification matrix The case sat under the suite that covers how calendar properties are read, but it cannot detect a regression there: a property reader that returned nothing would short-circuit on the missing UID and produce the same null. Moved to the classification matrix, where the verdict it checks is the point. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GcnBUEdW86ovpv1pT2d3b3 --- .../apple/src/mail/calendar-bundle.test.ts | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/connectors/apple/src/mail/calendar-bundle.test.ts b/connectors/apple/src/mail/calendar-bundle.test.ts index 61125403..39ce49bf 100644 --- a/connectors/apple/src/mail/calendar-bundle.test.ts +++ b/connectors/apple/src/mail/calendar-bundle.test.ts @@ -63,6 +63,22 @@ describe("classifyICS — the full classification matrix", () => { expect(result).toBeNull(); }); + it("skips a METHOD:REPLY with no SEQUENCE too — folding is not this function's job", () => { + // The null verdict says nothing about what becomes of the message: in a + // sync pass a recognised response is folded onto the event's own thread + // before it is ever offered here (see `sync.ts`). + const raw = [ + "BEGIN:VCALENDAR", + "METHOD:REPLY", + "BEGIN:VEVENT", + "UID:evt-reply@example.test", + "END:VEVENT", + "END:VCALENDAR", + ].join("\r\n"); + + expect(classifyICS(raw)).toBeNull(); + }); + it("returns null when the ICS has no UID at all", () => { const result = classifyICS(ics({ method: "CANCEL" })); expect(result).toBeNull(); @@ -144,17 +160,4 @@ describe("classifyICS — property reading after the shared-icsProp swap", () => expect(classifyICS(ics)).toEqual({ uid: "evt-params@example.test", kind: "update" }); }); - - it("still skips a METHOD:REPLY (folding is not this function's job)", () => { - const ics = [ - "BEGIN:VCALENDAR", - "METHOD:REPLY", - "BEGIN:VEVENT", - "UID:evt-reply@example.test", - "END:VEVENT", - "END:VCALENDAR", - ].join("\r\n"); - - expect(classifyICS(ics)).toBeNull(); - }); });