fix: keep the reader's read state when the conversation loads after marking it read

src/lib/__tests__/email-detail-cache.test.ts

@@ -320,6 +320,49 @@ describe('loading a conversation', () => {
     expect(threadGet).toHaveBeenCalledWith(['t1'], 'group');
   });
 
+  it('keeps a read mark made while the conversation was loading', async () => {
+    fullGet.mockResolvedValueOnce(fullResponse([full('m1')], 's1'));
+    await loadDetail('m1');
+    let answer!: (v: unknown) => void;
+    threadGet.mockReturnValueOnce(new Promise((r) => { answer = r; }));
+    const loading = loadThread('t-m1');
+    await flush();
+    // Marked read on open, while Thread/get is out.
+    patchDetail('m1', undefined, { keywords: { $seen: true } });
+    answer(threadResponse('t-m1', [{ id: 'm1', threadId: 't-m1', keywords: {}, mailboxIds: { inbox: true } }], 's2'));
+    await loading;
+
+    expect(peekDetail('m1')?.keywords).toEqual({ $seen: true });
+    expect(peekThread('t-m1')?.headers.get('m1')?.keywords).toEqual({ $seen: true });
+  });
+
+  it('keeps a read mark made while the message itself was loading', async () => {
+    let answer!: (v: unknown) => void;
+    fullGet.mockReturnValueOnce(new Promise((r) => { answer = r; }));
+    threadGet.mockResolvedValueOnce(threadResponse('t-m1', [{ id: 'm1', threadId: 't-m1', keywords: {}, mailboxIds: { inbox: true } }]));
+    await loadThread('t-m1');
+    const loading = loadDetail('m1');
+    await flush();
+    patchDetail('m1', undefined, { keywords: { $seen: true } });
+    answer(fullResponse([full('m1')], 's2'));
+
+    expect((await loading).keywords).toEqual({ $seen: true });
+    expect(peekDetail('m1')?.keywords).toEqual({ $seen: true });
+  });
+
+  it('takes the server keywords of a conversation read after the local change', async () => {
+    fullGet.mockResolvedValueOnce(fullResponse([full('m1')], 's1'));
+    await loadDetail('m1');
+    patchDetail('m1', undefined, { keywords: { $seen: true } });
+    threadGet.mockResolvedValueOnce(
+      threadResponse('t-m1', [{ id: 'm1', threadId: 't-m1', keywords: { $seen: true, $flagged: true }, mailboxIds: { inbox: true } }], 's2'),
+    );
+
+    await loadThread('t-m1');
+
+    expect(peekDetail('m1')?.keywords).toEqual({ $seen: true, $flagged: true });
+  });
+
   it('applies local keyword changes to the conversation headers too', async () => {
     threadGet.mockResolvedValue(threadResponse('t1', headers(['m1'])));
     await loadThread('t1');

src/lib/email-detail-cache.ts

@@ -163,6 +163,20 @@ function putThread(key: string, entry: ThreadEntry): void {
   }
 }
 
+// Local changes (patchDetail) by message key, numbered in the order they
+// were made. A response read before one of them must not undo it: the read
+// mark set on open was put back by the conversation's headers arriving just
+// after it, and the reader's toolbar offered "Mark as read" again.
+let localClock = 0;
+const localPatches = new Map<string, { at: number; patch: FlagsHint }>();
+const MAX_LOCAL_PATCHES = 500;
+
+/** What was changed here on `key` after a request started at `startedAt` went out. */
+function localSince(key: string, startedAt: number): FlagsHint | undefined {
+  const local = localPatches.get(key);
+  return local && local.at > startedAt ? local.patch : undefined;
+}
+
 /** The copy held for a message, if any. Synchronous: for rendering. */
 export function peekDetail(id: string, accountId?: string): Email | undefined {
   return details.get(keyOf(id, accountId))?.email;
@@ -185,6 +199,13 @@ export function subscribeEmailCache(listener: () => void): () => void {
  */
 export function patchDetail(id: string, accountId: string | undefined, patch: FlagsHint): void {
   const key = keyOf(id, accountId);
+  // Re-inserted, so the oldest change is the first to go.
+  const earlier = localPatches.get(key)?.patch;
+  localPatches.delete(key);
+  localPatches.set(key, { at: ++localClock, patch: { ...earlier, ...patch } });
+  while (localPatches.size > MAX_LOCAL_PATCHES) {
+    localPatches.delete(localPatches.keys().next().value as string);
+  }
   let changed = false;
   const entry = details.get(key);
   if (entry) {
@@ -217,6 +238,7 @@ export function clearEmailDetailCache(): void {
   pendingThreads.clear();
   threadBatches.clear();
   latestStates.clear();
+  localPatches.clear();
   detailChars = 0;
 }
 
@@ -252,12 +274,15 @@ async function fetchBodies(
 ): Promise<void> {
   if (ids.length === 0) return;
   const gen = generation;
+  const startedAt = localClock;
   try {
     const res = await getFullEmailsWithState(ids, accountId);
     const keep = gen === generation;
     if (keep) noteEmailState(accountId, res.state);
     const found = new Map<string, Email>();
-    for (const email of res.list) {
+    for (const fetched of res.list) {
+      const local = localSince(keyOf(fetched.id, accountId), startedAt);
+      const email = local ? { ...fetched, ...local } : fetched;
       found.set(email.id, email);
       if (!keep) continue;
       putDetail(keyOf(email.id, accountId), { email, state: res.state, chars: bodyChars(email) });
@@ -281,6 +306,7 @@ async function revalidate(
 ): Promise<void> {
   if (ids.length === 0) return;
   const gen = generation;
+  const startedAt = localClock;
   let gone = new Set<string>();
   try {
     const res = await getEmailFlags(ids, accountId);
@@ -288,10 +314,11 @@ async function revalidate(
     noteEmailState(accountId, res.state);
     gone = new Set(res.notFound);
     let changed = false;
-    for (const flags of res.list) {
+    for (const read of res.list) {
-      const key = keyOf(flags.id, accountId);
+      const key = keyOf(read.id, accountId);
       const entry = details.get(key);
       if (!entry) continue;
+      const flags = { ...read, ...localSince(key, startedAt) };
       const same = sameFlags(flags.keywords, entry.email.keywords)
         && sameFlags(flags.mailboxIds, entry.email.mailboxIds);
       // Keep the object when nothing changed, so nothing re-renders.
@@ -471,6 +498,7 @@ async function fetchThreads({ accountId, waiting }: ThreadBatch): Promise<void>
   const gen = generation;
   // What was held when the load started: shown again if the refetch fails.
   const fallback = (threadId: string) => (gen === generation ? threads.get(keyOf(threadId, accountId)) : undefined);
+  const startedAt = localClock;
   try {
     const res = await getThreadsHeaders(Array.from(waiting.keys()), accountId);
     if (gen !== generation) throw new Error('cleared');
@@ -483,15 +511,20 @@ async function fetchThreads({ accountId, waiting }: ThreadBatch): Promise<void>
         else d.reject(new Error(`Thread ${threadId} not found`));
         continue;
       }
+      // A change made here while the headers were on their way stays.
+      const list = found.list.map((e) => {
+        const local = localSince(keyOf(e.id, accountId), startedAt);
+        return local ? { ...e, ...local } : e;
+      });
       const entry: ThreadEntry = {
         ids: found.emailIds,
-        headers: new Map(found.list.map((e) => [e.id, e])),
+        headers: new Map(list.map((e) => [e.id, e])),
         state: res.state,
       };
       putThread(keyOf(threadId, accountId), entry);
       // The headers were just read: a member's copy held from an earlier
       // open takes their keywords and folders, which may have changed since.
-      for (const header of found.list) {
+      for (const header of list) {
         const memberKey = keyOf(header.id, accountId);
         const held = details.get(memberKey);
         if (!held) continue;