diff --git a/web/src/store/__tests__/reply-addressing.test.ts b/web/src/store/__tests__/reply-addressing.test.ts new file mode 100644 index 0000000..d754e5d --- /dev/null +++ b/web/src/store/__tests__/reply-addressing.test.ts @@ -0,0 +1,165 @@ +import { beforeEach, afterEach, describe, expect, it } from "vitest"; +import { useCompose } from "@/store/compose"; +import { useMail } from "@/store/mail"; +import type { Email, Identity } from "@/jmap/types"; + +/* + * Who a reply is addressed to. + * + * The hard half is replying to something *I* sent, which is what following up + * on your own last message is. The conversation is with the people I wrote to; + * addressing the reply to myself, or to my own Reply-To, sends it nowhere + * useful -- and on a Reply all it quietly demotes everyone I was talking to + * into Cc. + * + * There was a guard for this and it was sound. What it rested on was not: it + * asked whether an address was in the identity list, which is empty before + * identities load, misses an alias or a shared mailbox the server does not list + * as an identity, and compared strings where the rest of the codebase uses + * `sameAddress`. Each miss was silent. So the folder is asked first -- a + * message in Sent is mine whatever address it went out as -- and the identity + * list is the second opinion rather than the only one (#275). + */ + +const body = { + messageId: [""], subject: "Numbers", references: [], inReplyTo: [], + keywords: {}, htmlBody: [{ partId: "1", type: "text/html" }], textBody: [{ partId: "1", type: "text/html" }], + bodyValues: { "1": { value: "

hi

", isEncodingProblem: false, isTruncated: false } }, + attachments: [], receivedAt: "2026-09-04T10:00:00Z", mailboxIds: {}, +}; + +const ME = { name: "John", email: "john@example.org" }; +const ANN = { name: "Ann", email: "ann@example.com" }; +const BOB = { name: "Bob", email: "bob@example.com" }; + +/** A message I sent: me in From, Ann in To, Bob in Cc. */ +const MINE = { ...body, id: "m1", from: [ME], to: [ANN], cc: [BOB] } as unknown as Email; +/** The same conversation, but Ann's message to me. */ +const HERS = { ...body, id: "m2", from: [ANN], to: [ME], cc: [BOB] } as unknown as Email; + +const IDENTITIES = [{ id: "i1", name: "John", email: "john@example.org", replyTo: null }] as unknown as Identity[]; + +/** In Sent, which is the signal that survives an unlisted alias. */ +const inSent = (e: Email) => ({ ...e, mailboxIds: { sent1: true } }) as Email; + +function draftFor(email: Email, mode: "reply" | "replyAll" | "forward", opts: { identities?: Identity[] } = {}) { + const identities = opts.identities ?? IDENTITIES; + useMail.setState({ + accountId: "a1", + identities: identities as never, + getEmails: (async () => [email]) as never, + defaultIdentity: (() => identities[0]) as never, + loadIdentities: (async () => identities) as never, + roleId: ((role: string) => (role === "sent" ? "sent1" : null)) as never, + }); + return useCompose.getState().reply(email, mode).then((key) => useCompose.getState().drafts.find((d) => d.key === key)!); +} + +const addrs = (list: { email: string }[]) => list.map((a) => a.email); + +beforeEach(() => useCompose.setState({ drafts: [], activeKey: null })); +afterEach(() => useCompose.setState({ drafts: [], activeKey: null })); + +describe("replying to a message somebody sent me", () => { + it("replies to the sender", async () => { + const d = await draftFor(HERS, "reply"); + expect(addrs(d.to)).toEqual([ANN.email]); + expect(d.cc).toEqual([]); + }); + + it("reply all keeps the others and leaves me off", async () => { + const d = await draftFor(HERS, "replyAll"); + expect(addrs(d.to)).toEqual([ANN.email]); + expect(addrs(d.cc)).toEqual([BOB.email]); + }); + + it("honours the sender's Reply-To, which is what it is for", async () => { + const d = await draftFor({ ...HERS, replyTo: [{ name: null, email: "desk@example.com" }] } as Email, "reply"); + expect(addrs(d.to)).toEqual(["desk@example.com"]); + }); +}); + +describe("replying to a message I sent", () => { + it("writes to the people I wrote to, not to me", async () => { + const d = await draftFor(MINE, "reply"); + expect(addrs(d.to)).toEqual([ANN.email]); + expect(d.cc).toEqual([]); + }); + + it("reply all keeps my Cc as Cc, rather than promoting me into To", async () => { + const d = await draftFor(MINE, "replyAll"); + expect(addrs(d.to)).toEqual([ANN.email]); + expect(addrs(d.cc)).toEqual([BOB.email]); + }); + + it("does not follow my own Reply-To back to my own desk", async () => { + // The address replies to *me* belong at. My reply is not one of them. + const d = await draftFor({ ...MINE, replyTo: [{ name: null, email: "desk@example.org" }] } as Email, "replyAll"); + expect(addrs(d.to)).toEqual([ANN.email]); + expect(addrs(d.cc)).toEqual([BOB.email]); + }); + + it("leaves me out even when I was a recipient of my own message", async () => { + const d = await draftFor({ ...MINE, to: [ME, ANN] } as Email, "replyAll"); + expect(addrs(d.to)).toEqual([ANN.email]); + expect(addrs(d.cc)).toEqual([BOB.email]); + }); + + it("recognises my address however the identity stored it", async () => { + // A hand-typed identity address can carry whitespace, and comparing + // strings rather than addresses made that enough to break the reply. + const padded = [{ id: "i1", name: "John", email: " John@Example.ORG " }] as unknown as Identity[]; + const d = await draftFor(MINE, "replyAll", { identities: padded }); + expect(addrs(d.to)).toEqual([ANN.email]); + }); +}); + +describe("when the identity list cannot answer", () => { + it("takes a message in Sent as mine, whatever address it went out as", async () => { + // An alias or a shared mailbox the server does not list as an identity. + const alias = inSent({ ...MINE, from: [{ name: "Sales", email: "sales@example.org" }] } as Email); + const d = await draftFor(alias, "replyAll"); + expect(addrs(d.to)).toEqual([ANN.email]); + expect(addrs(d.cc)).toEqual([BOB.email]); + }); + + it("takes a message in Sent as mine before the identities have loaded", async () => { + const d = await draftFor(inSent(MINE), "replyAll", { identities: [] }); + expect(addrs(d.to)).toEqual([ANN.email]); + }); + + it("still replies to the sender of a message that is not mine and not in Sent", async () => { + // The folder signal must not swallow the ordinary case when it is absent. + const d = await draftFor(HERS, "replyAll", { identities: [] }); + expect(addrs(d.to)).toEqual([ANN.email]); + }); +}); + +describe("a message of mine with nobody obvious to reply to", () => { + it("uses the Cc when I addressed it to nobody else", async () => { + const d = await draftFor({ ...MINE, to: [ME] } as Email, "replyAll"); + expect(addrs(d.to)).toEqual([BOB.email]); + expect(d.cc).toEqual([]); + }); + + it("uses the Cc on a plain reply too, rather than leaving To empty", async () => { + const d = await draftFor({ ...MINE, to: [] } as Email, "reply"); + expect(addrs(d.to)).toEqual([BOB.email]); + }); + + it("falls back to my own address rather than a draft addressed to nobody", async () => { + // A note I sent only to myself. Replying to it is odd, and an empty To is + // worse than the only address there was. + const d = await draftFor({ ...MINE, to: [ME], cc: [] } as Email, "reply"); + expect(addrs(d.to)).toEqual([ME.email]); + }); +}); + +describe("forwarding", () => { + it("addresses nobody, whoever sent the message", async () => { + for (const m of [MINE, HERS]) { + const d = await draftFor(m, "forward"); + expect([d.to, d.cc]).toEqual([[], []]); + } + }); +}); diff --git a/web/src/store/compose.ts b/web/src/store/compose.ts index 5688713..7776145 100644 --- a/web/src/store/compose.ts +++ b/web/src/store/compose.ts @@ -299,26 +299,50 @@ export const useCompose = create((set, get) => ({ const full = (await mail.getEmails([email.id], true))[0] ?? email; const identities = mail.identities.length ? mail.identities : await mail.loadIdentities(); const ident = defaultIdentity(identities, full); - const ownEmails = identities.map((i) => i.email.toLowerCase()); - const isOwn = (a: EmailAddress) => ownEmails.includes(a.email.toLowerCase()); + const ownEmails = identities.map((i) => i.email); + /* `sameAddress` rather than a lowercased `includes`, because an identity + address can carry whitespace and a hand-typed one does. */ + const isOwn = (a: EmailAddress) => ownEmails.some((e) => sameAddress(e, a.email)); + const withoutOwn = (list: EmailAddress[]) => uniqueAddresses(list).filter((a) => !isOwn(a)); const s = settings(); + /* + * Was this message mine? + * + * The folder answers it before the addresses do, and has to: the address + * test fails in exactly the cases where the mistake is least visible. A + * message sent from an alias or a shared mailbox that `Identity/get` does + * not list is not recognisably mine, and neither is anything at all if the + * identities have not loaded yet -- and the failure is silent, addressing + * the reply back to me with everyone I actually wrote to moved to Cc. + * + * A message in Sent is mine whatever address it went out as. + */ + const sentId = mail.roleId("sent"); + const sentByMe = (Boolean(full.from?.length) && (full.from ?? []).every(isOwn)) + || Boolean(sentId && full.mailboxIds?.[sentId]); + let to: EmailAddress[] = []; let cc: EmailAddress[] = []; if (mode === "reply" || mode === "replyAll") { - const replyTo = full.replyTo?.length ? full.replyTo : (full.from ?? []); - to = uniqueAddresses(replyTo); - if (mode === "replyAll") { - const others = uniqueAddresses([...(full.to ?? []), ...(full.cc ?? [])]).filter((a) => !isOwn(a) && !to.some((t) => sameAddress(t.email, a.email))); - cc = others; - // If the message was sent by me, reply to original recipients instead. - if (to.every(isOwn) && full.to?.length) { - to = uniqueAddresses(full.to); - cc = uniqueAddresses(full.cc ?? []).filter((a) => !isOwn(a)); + if (sentByMe && (full.to?.length || full.cc?.length)) { + /* + * Replying to something I sent continues the conversation with the + * people I wrote to. Not with myself, and not with my own Reply-To + * either -- that address is where replies *to me* belong, and following + * it here would send my own reply to my own desk. + */ + to = withoutOwn(full.to ?? []); + cc = mode === "replyAll" ? withoutOwn(full.cc ?? []) : []; + // Addressed only to myself, or only in Cc: there is still somebody this + // is a reply to, and an empty To is not it. + if (!to.length) { to = cc.length ? cc : withoutOwn(full.cc ?? []); cc = []; } + if (!to.length) to = uniqueAddresses([...(full.to ?? []), ...(full.cc ?? [])]); + } else { + to = uniqueAddresses(full.replyTo?.length ? full.replyTo : (full.from ?? [])); + if (mode === "replyAll") { + cc = uniqueAddresses([...(full.to ?? []), ...(full.cc ?? [])]).filter((a) => !isOwn(a) && !to.some((t) => sameAddress(t.email, a.email))); } - } else if (to.every(isOwn) && full.to?.length) { - to = uniqueAddresses(full.to.filter((a) => !isOwn(a))); - if (!to.length) to = uniqueAddresses(full.to); } }