From bfe95e0ebd69acbc43a9c189d306108bdf7ab59e Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 17 Oct 2026 00:28:57 +0200 Subject: [PATCH] fix: keep your own address out of reply-all before your identities have loaded #803 --- .../email/__tests__/reply-addressing.test.tsx | 56 ++++++++++++++++++- components/email/email-composer.tsx | 42 +++++++++++++- lib/__tests__/reply-recipients.test.ts | 28 +++++++++- lib/reply-recipients.ts | 25 +++++++++ 4 files changed, 147 insertions(+), 4 deletions(-) diff --git a/components/email/__tests__/reply-addressing.test.tsx b/components/email/__tests__/reply-addressing.test.tsx index f8afaa5e8..8a489d537 100644 --- a/components/email/__tests__/reply-addressing.test.tsx +++ b/components/email/__tests__/reply-addressing.test.tsx @@ -1,10 +1,11 @@ -import { render, screen } from '@testing-library/react'; +import { render, screen, fireEvent } from '@testing-library/react'; import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import React from 'react'; import { EmailComposer } from '../email-composer'; import { useSettingsStore } from '@/stores/settings-store'; import { useAuthStore } from '@/stores/auth-store'; import { useAccountStore } from '@/stores/account-store'; +import { useIdentityStore } from '@/stores/identity-store'; import { usePolicyStore } from '@/stores/policy-store'; // ─── Heavy component mocks (mirrors recipient-paste.test.tsx) ───────────────── @@ -491,6 +492,59 @@ describe('composer reply addressing', () => { }); }); +// #803: identities load from the server after the app starts and aren't +// persisted. A reply-all opened before they arrive used to keep our own +// address in To, and nothing fixed it once they loaded. +describe('reply-all before identities have loaded', () => { + const identityStore = useIdentityStore as unknown as { + getState: () => { identities: unknown[] }; + setState: (p: Record) => void; + }; + const accountStore = useAccountStore as unknown as { setState: (p: Record) => void }; + const authStore = useAuthStore as unknown as { setState: (p: Record) => void }; + let loaded: unknown[]; + + beforeEach(() => { + loaded = identityStore.getState().identities; + identityStore.setState({ identities: [] }); + }); + + afterEach(() => { + identityStore.setState({ identities: loaded }); + accountStore.setState({ accounts: [] }); + authStore.setState({ activeAccountId: null }); + }); + + it('drops our own address using the account address', () => { + accountStore.setState({ + accounts: [{ id: 'local-1', email: 'me@example.com', username: 'me@example.com', isConnected: true }], + }); + authStore.setState({ activeAccountId: 'local-1' }); + render(); + expect(toChips()).toEqual(['Bob (bob@other.com)', 'Carol (carol@other.com)']); + }); + + it('drops our own address once the identities arrive', () => { + const { rerender } = render(); + expect(toChips()).toContain('Me (me@example.com)'); + identityStore.setState({ identities: loaded }); + rerender(); + expect(toChips()).toEqual(['Bob (bob@other.com)', 'Carol (carol@other.com)']); + expect(ccChips()).toEqual(['Dave (dave@other.com)']); + }); + + it('leaves the recipients alone once the user has changed them', () => { + const { rerender } = render(); + const carol = Array.from(screen.getByTestId('composer-to').querySelectorAll('[draggable]')) + .find((el) => el.textContent?.includes('carol@other.com')) as HTMLElement; + fireEvent.click(carol.querySelector('button') as HTMLElement); + expect(toChips()).toEqual(['Bob (bob@other.com)', 'Me (me@example.com)']); + identityStore.setState({ identities: loaded }); + rerender(); + expect(toChips()).toEqual(['Bob (bob@other.com)', 'Me (me@example.com)']); + }); +}); + describe('composer @-mention candidates', () => { const offered = () => editorProps.mentionCandidates.map((c) => `${c.label} <${c.email}>`); diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 27a5b9e42..afda215ab 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -91,7 +91,7 @@ import type { EmailTemplate } from "@/lib/template-types"; import { appendPlainTextSignature, getPlainTextSignature, plainTextBodyHasSignature, plainTextBodyWithoutSignature } from "@/lib/signature-utils"; import { wantsSignatureSeparator } from "@/lib/signature-delimiter"; import { findComposeIdentityId, findDraftIdentityId, findReplyIdentityId, resolveReplyFrom } from "@/lib/reply-identity"; -import { buildReplyRecipients, isSelfSent } from "@/lib/reply-recipients"; +import { buildReplyRecipients, isSelfSent, ownReplyAddresses } from "@/lib/reply-recipients"; import { computeReplyThreadingHeaders, type ReplyThreadingHeaders } from "@/lib/email-threading"; import { RecipientsRejectedError, RequestTimeoutError, ScheduleTooLateError, SendUnconfirmedError, formatRejectedRecipients } from "@/lib/jmap/client"; import { @@ -520,7 +520,17 @@ export function EmailComposer({ const toRecipient = (r: { name?: string; email?: string }): Recipient => ({ name: r.name && r.name !== r.email ? r.name : undefined, email: r.email ?? "" }); - const ownIdentityEmails = identities.map(i => i.email).filter((e): e is string => Boolean(e)); + // Identities can still be loading when the composer opens (cold start, PWA + // relaunch), so the account's own addresses stand in until they arrive (#803). + const composerAccount = activeAccountId ? useAccountStore.getState().getAccountById(activeAccountId) : undefined; + const replyAccount = sourceAccountId && sourceAccountId !== activeAccountId + ? useAccountStore.getState().getAccountById(sourceAccountId) + : undefined; + const ownIdentityEmails = ownReplyAddresses( + identities.map(i => i.email), + [composerAccount?.email, composerAccount?.username, replyAccount?.email, replyAccount?.username], + ); + const ownIdentityEmailsKey = ownIdentityEmails.map((e) => e.toLowerCase()).join(','); // Initialize with reply/forward data if provided const getInitialTo = (): Recipient[] => { @@ -726,6 +736,34 @@ export function EmailComposer({ const [showCc, setShowCc] = useState(() => initialData?.showCc ?? (prefill?.cc ? prefill.cc.length > 0 : getInitialCc().length > 0)); const [showBcc, setShowBcc] = useState(initialData?.showBcc ?? false); + // The reply recipients worked out when the composer opened. When the + // identities arrive later (#803) they're worked out again, but only while + // To and Cc still hold exactly what was filled in, so nothing the user + // changed is overwritten. Drafts and prefilled composes are left alone. + const autoRecipientsRef = useRef<{ to: Recipient[]; cc: Recipient[] } | null>( + !initialData && !prefill && (mode === 'reply' || mode === 'replyAll') + ? { to, cc } + : null, + ); + useEffect(() => { + const auto = autoRecipientsRef.current; + if (!auto) return; + const same = (a: Recipient[], b: Recipient[]) => + a.length === b.length && a.every((r, i) => r.email === b[i].email && r.name === b[i].name); + if (!same(to, auto.to) || !same(cc, auto.cc)) { + autoRecipientsRef.current = null; + return; + } + const nextTo = getInitialTo(); + const nextCc = getInitialCc(); + if (same(nextTo, auto.to) && same(nextCc, auto.cc)) return; + autoRecipientsRef.current = { to: nextTo, cc: nextCc }; + setTo(nextTo); + setCc(nextCc); + if (nextCc.length > 0) setShowCc(true); + // Only a change in the user's own addresses should redo the recipients. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ownIdentityEmailsKey]); // Committed recipients plus any not-yet-committed text the user has typed. // Send/validation/draft paths treat a typed-but-uncommitted address as a // real recipient, matching the previous string-based behavior. diff --git a/lib/__tests__/reply-recipients.test.ts b/lib/__tests__/reply-recipients.test.ts index 66d2e2e72..c3966e514 100644 --- a/lib/__tests__/reply-recipients.test.ts +++ b/lib/__tests__/reply-recipients.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { buildReplyRecipients, isSelfSent } from '@/lib/reply-recipients'; +import { buildReplyRecipients, isSelfSent, ownReplyAddresses } from '@/lib/reply-recipients'; const OWN = ['me@example.com', 'info@example.com']; @@ -135,3 +135,29 @@ describe('buildReplyRecipients', () => { expect(isSelfSent(undefined, OWN)).toBe(false); }); }); + +// #803: identities load after the app starts, so a composer opened before +// they arrive had no own addresses and reply-all kept the user in To. +describe('ownReplyAddresses', () => { + const received = { + from: [{ email: 'bob@other.com' }], + to: [{ email: 'me@example.com' }, { email: 'carol@other.com' }], + cc: [{ email: 'Me@Example.com' }], + }; + + it('drops our own address on reply-all before identities have loaded', () => { + const own = ownReplyAddresses([], ['me@example.com', 'me']); + const { to, cc } = buildReplyRecipients(received, 'replyAll', own); + expect(emails(to)).toEqual(['bob@other.com', 'carol@other.com']); + expect(cc).toEqual([]); + }); + + it('merges identities with the account addresses, once each', () => { + expect(ownReplyAddresses(['me@example.com', undefined, ''], ['ME@example.com ', 'info@example.com'])) + .toEqual(['me@example.com', 'info@example.com']); + }); + + it('skips a login name without a domain', () => { + expect(ownReplyAddresses([], ['linus', null, undefined])).toEqual([]); + }); +}); diff --git a/lib/reply-recipients.ts b/lib/reply-recipients.ts index f98a50c2f..fdf260825 100644 --- a/lib/reply-recipients.ts +++ b/lib/reply-recipients.ts @@ -44,6 +44,31 @@ function isOwnAddress(email: string | undefined, ownEmails: string[]): boolean { return ownEmails.some((own) => normalizeBase(own) === base); } +/** + * The addresses a reply treats as the user's own: their identities plus the + * account's login and primary address. Identities load from the server after + * the app starts and aren't persisted, so a composer opened before they arrive + * saw none and reply-all kept the user in To/Cc (#803). The account's + * addresses are known from the start and cover the usual case until then. + * A bare login name without a domain can't match an address, so it's skipped. + */ +export function ownReplyAddresses( + identityEmails: (string | undefined | null)[], + accountAddresses: (string | undefined | null)[] = [], +): string[] { + const seen = new Set(); + const result: string[] = []; + for (const email of [...identityEmails, ...accountAddresses]) { + const trimmed = email?.trim(); + if (!trimmed || !trimmed.includes('@')) continue; + const key = normalize(trimmed); + if (seen.has(key)) continue; + seen.add(key); + result.push(trimmed); + } + return result; +} + /** * Is this a message the user themself sent? True when the From address is one * of their own identities - the case that shows up when browsing a thread and