coffey-labs/ihasmail
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 2 Wiki Activity

Ask the folder, not just the identity list, whether a message was mine

Browse Source 
Replying to a thread whose last message I sent addressed the reply to me:
Reply put my own address in To, and Reply all put me in To with everyone
I had actually written to demoted to Cc. Following up on your own last
message is an ordinary thing to do, and this made it useless.

There was already a guard for exactly this, and the guard was sound. What
it rested on was not. It asked whether an address was in the identity
list, and that question has a wrong answer in more situations than it has
a right one:

- the list is empty until identities load;
- an alias or a shared mailbox is not in it at all;
- it compared lowercased strings with `includes` where the rest of the
  codebase uses `sameAddress`, so an identity address stored with
  whitespace was enough to break it;
- the check ran on the address the reply was about to go to rather than
  on the sender, so a message of mine carrying a Reply-To skipped it
  entirely and my reply went to my own desk;
- and the Reply all branch never filtered my own address out of To, though
  the Reply branch did.

Every one of those failed silently, which is why five of them accumulated.

So the folder is asked first: a message in Sent is mine whatever address
it went out as, and `mailboxIds` is already fetched in LIST_PROPS with
roleId("sent") on the mail store, so this costs no request. The identity
list stays as a second opinion, now compared with `sameAddress`, and the
whole test keys off the sender rather than off the computed recipient.

Two cases remain unanswerable and are commented rather than papered over:
a message from an unlisted alias that is not in Sent either, and any
message at all when identities failed to load and it is not in Sent.
Neither signal exists. Both are far narrower than what was broken.

Reply addressing had no tests at all, which is how a guard this
load-bearing came to be wrong five ways at once. Fifteen now, seven of
which fail against the old code.
This commit is contained in:
jcoffey-dev
2026-09-04 08:10:28 -07:00
parent 4d23cef511
commit 029f079094
2 changed files with 203 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/store/__tests__/reply-addressing.test.ts
+165
View File
@@ -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: ["<[email protected]>"], subject: "Numbers", references: [], inReplyTo: [],
keywords: {}, htmlBody: [{ partId: "1", type: "text/html" }], textBody: [{ partId: "1", type: "text/html" }],
bodyValues: { "1": { value: "<p>hi</p>", isEncodingProblem: false, isTruncated: false } },
attachments: [], receivedAt: "2026-09-04T10:00:00Z", mailboxIds: {},
};
const ME = { name: "John", email: "[email protected]" };
const ANN = { name: "Ann", email: "[email protected]" };
const BOB = { name: "Bob", email: "[email protected]" };
/** 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: "[email protected]", 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: "[email protected]" }] } as Email, "reply");
expect(addrs(d.to)).toEqual(["[email protected]"]);
});
});
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: "[email protected]" }] } 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: " [email protected] " }] 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: "[email protected]" }] } 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([[], []]);
}
});
});
web/src/store/compose.ts
+38 -14
View File
@@ -299,26 +299,50 @@ export const useCompose = create<ComposeState>((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);
}
}