fix(chats): let an invalidated list read survive marking a chat read

Mark read and unread cancel list reads to protect their optimistic flag, and
the flag's write clears isInvalidated, so a read owed by an invalidation never
ran again. A conversation mounting just after its chat was deleted did exactly
that and left the deleted row in the sidebar. Read the list again when one was
cancelled.

Author: waleedlatif1

apps/sim/hooks/queries/mothership-chats.test.ts

@@ -1,3 +1,5 @@
+import { flushMicrotasks } from '@sim/testing/helpers/async'
+import { createDeferred } from '@sim/testing/helpers/deferred'
 import { jsonResponse } from '@sim/testing/helpers/http'
 import { reactQueryMock, reactQueryMockFns } from '@sim/testing/mocks/react-query.mock'
 import { sleep } from '@sim/utils/helpers'
@@ -27,7 +29,10 @@ vi.mock('@/lib/terminal/transport', () => ({
 import type { MothershipEffort } from '@/lib/mothership/model-options'
 import {
   fetchMothershipChatHistory,
+  mothershipChatKeys,
   useDeleteMothershipChats,
+  useMarkMothershipChatRead,
+  useMarkMothershipChatUnread,
   useSetMothershipChatEffort,
 } from '@/hooks/queries/mothership-chats'
 import { useMothershipEffortStore } from '@/stores/mothership-effort/store'
@@ -101,6 +106,61 @@ describe('tasks query boundary parsing', () => {
     })
   })
 
+  it.each([
+    ['read', useMarkMothershipChatRead],
+    ['unread', useMarkMothershipChatUnread],
+  ] as const)(
+    'marking a chat %s still removes a row an invalidated list read would drop',
+    async (_name, useMark) => {
+      const tanstack =
+        await vi.importActual<typeof import('@tanstack/react-query')>('@tanstack/react-query')
+      const client = new tanstack.QueryClient()
+      const owner = { organizationId: 'org-1' }
+      const queryKey = mothershipChatKeys.ownerList(owner)
+      const rows = [{ id: 'chat-gone', name: 'Deleted', isUnread: false }]
+      const staleRead = createDeferred<typeof rows>()
+      const queryFn = vi
+        .fn<() => Promise<typeof rows>>()
+        .mockReturnValueOnce(staleRead.promise)
+        .mockResolvedValue([])
+      client.setQueryData(queryKey, rows)
+      const list = new tanstack.QueryObserver(client, { queryKey, queryFn, staleTime: 60_000 })
+      const stop = list.subscribe(() => {})
+      reactQueryMockFns.mockUseQueryClient.mockReturnValueOnce(
+        client as unknown as typeof queryClient
+      )
+      const mutation = useMark(owner) as unknown as {
+        onMutate: (chatId: string) => Promise<unknown>
+        onSettled: (data: undefined, error: null, chatId: string, context: unknown) => void
+      }
+
+      void client.invalidateQueries({ queryKey })
+      const context = await mutation.onMutate('chat-gone')
+      mutation.onSettled(undefined, null, 'chat-gone', context)
+      await flushMicrotasks(10)
+
+      expect(queryFn).toHaveBeenCalledTimes(2)
+      expect(list.getCurrentResult().data).toEqual([])
+      stop()
+      client.clear()
+    }
+  )
+
+  it('leaves the list alone when marking a chat with no invalidated read to redo', async () => {
+    const owner = { organizationId: 'org-1' }
+    queryClient.getQueryData.mockReturnValueOnce([])
+    queryClient.getQueryState.mockReturnValueOnce({ isInvalidated: false })
+    const mutation = useMarkMothershipChatRead(owner) as unknown as {
+      onMutate: (chatId: string) => Promise<unknown>
+      onSettled: (data: undefined, error: null, chatId: string, context: unknown) => void
+    }
+
+    const context = await mutation.onMutate('chat-1')
+    mutation.onSettled(undefined, null, 'chat-1', context)
+
+    expect(queryClient.invalidateQueries).not.toHaveBeenCalled()
+  })
+
   it('loads a chat from a server that predates the effort field', async () => {
     vi.mocked(fetch).mockResolvedValueOnce(
       jsonResponse({

apps/sim/hooks/queries/mothership-chats.ts

@@ -660,6 +660,20 @@ function applyUnreadFlag(
   )
 }
 
+/**
+ * Stops the list reads that could overwrite an optimistic flag. Answers whether one was owed by an
+ * invalidation: the flag's write clears that mark, so the caller has to read the list again.
+ */
+async function holdChatListReads(
+  queryClient: ReturnType<typeof useQueryClient>,
+  owner: MothershipChatOwner | undefined
+): Promise<boolean> {
+  const queryKey = mothershipChatKeys.ownerList(owner)
+  const owesRead = queryClient.getQueryState(queryKey)?.isInvalidated === true
+  await queryClient.cancelQueries({ queryKey })
+  return owesRead
+}
+
 /**
  * Marks a chat as read with optimistic update.
  *
@@ -670,7 +684,8 @@ function applyUnreadFlag(
  * If there is no cached list yet (initial fetch still in flight, e.g. on
  * chat-page refresh), we skip cancellation entirely so the in-flight fetch
  * can resolve normally — otherwise it would be orphaned and never refetched.
- * `onSuccess` then reconciles whichever state the fetch produced.
+ * `onSuccess` then reconciles whichever state the fetch produced. A read an invalidation
+ * asked for still runs: cancelling one for good would keep a deleted chat in the sidebar.
  */
 export function useMarkMothershipChatRead(owner?: MothershipChatOwner) {
   const queryClient = useQueryClient()
@@ -680,12 +695,12 @@ export function useMarkMothershipChatRead(owner?: MothershipChatOwner) {
       const previousChats = queryClient.getQueryData<MothershipChatMetadata[]>(
         mothershipChatKeys.ownerList(owner)
       )
-      if (!previousChats) return { previousChats }
+      if (!previousChats) return { previousChats, owesRead: false }
 
-      await queryClient.cancelQueries({ queryKey: mothershipChatKeys.ownerList(owner) })
+      const owesRead = await holdChatListReads(queryClient, owner)
       applyUnreadFlag(queryClient, owner, chatId, false)
 
-      return { previousChats }
+      return { previousChats, owesRead }
     },
     onSuccess: (_data, chatId) => {
       applyUnreadFlag(queryClient, owner, chatId, false)
@@ -695,6 +710,10 @@ export function useMarkMothershipChatRead(owner?: MothershipChatOwner) {
         queryClient.setQueryData(mothershipChatKeys.ownerList(owner), context.previousChats)
       }
     },
+    onSettled: (_data, _error, _variables, context) => {
+      if (context?.owesRead)
+        queryClient.invalidateQueries({ queryKey: mothershipChatKeys.ownerList(owner) })
+    },
   })
 }
 
@@ -712,12 +731,12 @@ export function useMarkMothershipChatUnread(owner?: MothershipChatOwner) {
       const previousChats = queryClient.getQueryData<MothershipChatMetadata[]>(
         mothershipChatKeys.ownerList(owner)
       )
-      if (!previousChats) return { previousChats }
+      if (!previousChats) return { previousChats, owesRead: false }
 
-      await queryClient.cancelQueries({ queryKey: mothershipChatKeys.ownerList(owner) })
+      const owesRead = await holdChatListReads(queryClient, owner)
       applyUnreadFlag(queryClient, owner, chatId, true)
 
-      return { previousChats }
+      return { previousChats, owesRead }
     },
     onSuccess: (_data, chatId) => {
       applyUnreadFlag(queryClient, owner, chatId, true)
@@ -727,6 +746,10 @@ export function useMarkMothershipChatUnread(owner?: MothershipChatOwner) {
         queryClient.setQueryData(mothershipChatKeys.ownerList(owner), context.previousChats)
       }
     },
+    onSettled: (_data, _error, _variables, context) => {
+      if (context?.owesRead)
+        queryClient.invalidateQueries({ queryKey: mothershipChatKeys.ownerList(owner) })
+    },
   })
 }