Coalesce renderer worktree change refreshes (#6020)

Co-authored-by: Neil <neil@stably.ai>

src/renderer/src/hooks/useIpcEvents.ts

@@ -78,6 +78,7 @@ import {
   releaseBrowserAutomationVisibility
 } from '@/components/browser-pane/browser-automation-visibility'
 import { attachMobileMarkdownBridge } from '@/runtime/mobile-markdown-bridge'
+import { createWorktreeChangeRefreshQueue } from './worktree-change-refresh-queue'
 import { subscribeRuntimeClientEvents } from '@/runtime/runtime-client-events'
 import { createRuntimeClientEventsSync } from './runtime-client-events-sync'
 import { detectLanguage } from '@/lib/language-detect'
@@ -807,6 +808,7 @@ export function useIpcEvents(): void {
         afterState.removeWorkspaceSpaceWorktrees(removed)
       }
     }
+    const worktreeChangeRefreshQueue = createWorktreeChangeRefreshQueue(handleWorktreesChanged)
 
     const activateNotifiedWorktree = async (
       {
@@ -867,7 +869,7 @@ export function useIpcEvents(): void {
       }
       if (event.type === 'worktreesChanged') {
         void ensureRuntimeEventRepoKnown(environmentId, event.repoId).then(() =>
-          handleWorktreesChanged(event.repoId)
+          worktreeChangeRefreshQueue.enqueue({ repoId: event.repoId })
         )
         return
       }
@@ -935,7 +937,7 @@ export function useIpcEvents(): void {
           }
           // A folder rename changes the worktree id; handleWorktreesChanged
           // re-keys state and shields it from the deletion diff (see there).
-          await handleWorktreesChanged(data.repoId, data.renamed)
+          worktreeChangeRefreshQueue.enqueue(data)
         }
       )
     )

src/renderer/src/hooks/worktree-change-refresh-queue.test.ts

@@ -0,0 +1,142 @@
+import { describe, expect, it, vi } from 'vitest'
+import { createWorktreeChangeRefreshQueue } from './worktree-change-refresh-queue'
+
+function deferred<T = void>(): {
+  promise: Promise<T>
+  resolve: (value: T | PromiseLike<T>) => void
+} {
+  let resolve!: (value: T | PromiseLike<T>) => void
+  const promise = new Promise<T>((done) => {
+    resolve = done
+  })
+  return { promise, resolve }
+}
+
+async function flushPromises(): Promise<void> {
+  await Promise.resolve()
+  await Promise.resolve()
+}
+
+describe('createWorktreeChangeRefreshQueue', () => {
+  it('coalesces same-repo change bursts behind the active refresh', async () => {
+    const firstRefresh = deferred()
+    const handler = vi.fn(() => firstRefresh.promise)
+    const queue = createWorktreeChangeRefreshQueue(handler)
+
+    queue.enqueue({ repoId: 'repo-1' })
+    queue.enqueue({ repoId: 'repo-1' })
+    queue.enqueue({ repoId: 'repo-1' })
+
+    expect(handler).toHaveBeenCalledTimes(1)
+    expect(handler).toHaveBeenCalledWith('repo-1', undefined)
+
+    firstRefresh.resolve()
+    await flushPromises()
+
+    expect(handler).toHaveBeenCalledTimes(2)
+    expect(handler).toHaveBeenNthCalledWith(2, 'repo-1', undefined)
+  })
+
+  it('does not overlap refreshes for the same repo', async () => {
+    const firstRefresh = deferred()
+    const secondRefresh = deferred()
+    const handler = vi
+      .fn()
+      .mockReturnValueOnce(firstRefresh.promise)
+      .mockReturnValueOnce(secondRefresh.promise)
+    const queue = createWorktreeChangeRefreshQueue(handler)
+
+    queue.enqueue({ repoId: 'repo-1' })
+    queue.enqueue({ repoId: 'repo-1' })
+
+    expect(handler).toHaveBeenCalledTimes(1)
+
+    firstRefresh.resolve()
+    await flushPromises()
+
+    expect(handler).toHaveBeenCalledTimes(2)
+
+    secondRefresh.resolve()
+    await flushPromises()
+
+    expect(handler).toHaveBeenCalledTimes(2)
+  })
+
+  it('runs different repos independently', async () => {
+    const repoOneRefresh = deferred()
+    const handler = vi.fn((repoId: string) =>
+      repoId === 'repo-1' ? repoOneRefresh.promise : Promise.resolve()
+    )
+    const queue = createWorktreeChangeRefreshQueue(handler)
+
+    queue.enqueue({ repoId: 'repo-1' })
+    queue.enqueue({ repoId: 'repo-2' })
+
+    expect(handler).toHaveBeenCalledTimes(2)
+    expect(handler).toHaveBeenNthCalledWith(1, 'repo-1', undefined)
+    expect(handler).toHaveBeenNthCalledWith(2, 'repo-2', undefined)
+
+    repoOneRefresh.resolve()
+    await flushPromises()
+  })
+
+  it('continues draining queued refreshes after a failed refresh', async () => {
+    const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
+    try {
+      const firstRefresh = deferred()
+      const handler = vi
+        .fn()
+        .mockReturnValueOnce(firstRefresh.promise)
+        .mockRejectedValueOnce(new Error('refresh failed'))
+        .mockResolvedValue(undefined)
+      const queue = createWorktreeChangeRefreshQueue(handler)
+      const renamed = { oldWorktreeId: 'wt-old', newWorktreeId: 'wt-new' }
+
+      queue.enqueue({ repoId: 'repo-1' })
+      queue.enqueue({ repoId: 'repo-1', renamed })
+      queue.enqueue({ repoId: 'repo-1' })
+
+      firstRefresh.resolve()
+      await flushPromises()
+
+      expect(handler).toHaveBeenCalledTimes(3)
+      expect(handler).toHaveBeenNthCalledWith(2, 'repo-1', renamed)
+      expect(handler).toHaveBeenNthCalledWith(3, 'repo-1', undefined)
+    } finally {
+      consoleError.mockRestore()
+    }
+  })
+
+  it('preserves rename events instead of folding them into plain refreshes', async () => {
+    const handler = vi.fn(() => Promise.resolve())
+    const queue = createWorktreeChangeRefreshQueue(handler)
+    const renamed = { oldWorktreeId: 'wt-old', newWorktreeId: 'wt-new' }
+
+    queue.enqueue({ repoId: 'repo-1' })
+    queue.enqueue({ repoId: 'repo-1', renamed })
+    await flushPromises()
+
+    expect(handler).toHaveBeenNthCalledWith(1, 'repo-1', undefined)
+    expect(handler).toHaveBeenNthCalledWith(2, 'repo-1', renamed)
+  })
+
+  it('keeps a plain refresh queued after a rename', async () => {
+    const firstRefresh = deferred()
+    const handler = vi.fn().mockReturnValueOnce(firstRefresh.promise).mockResolvedValue(undefined)
+    const queue = createWorktreeChangeRefreshQueue(handler)
+    const renamed = { oldWorktreeId: 'wt-old', newWorktreeId: 'wt-new' }
+
+    queue.enqueue({ repoId: 'repo-1' })
+    queue.enqueue({ repoId: 'repo-1', renamed })
+    queue.enqueue({ repoId: 'repo-1' })
+
+    expect(handler).toHaveBeenCalledTimes(1)
+
+    firstRefresh.resolve()
+    await flushPromises()
+
+    expect(handler).toHaveBeenCalledTimes(3)
+    expect(handler).toHaveBeenNthCalledWith(2, 'repo-1', renamed)
+    expect(handler).toHaveBeenNthCalledWith(3, 'repo-1', undefined)
+  })
+})

src/renderer/src/hooks/worktree-change-refresh-queue.ts

@@ -0,0 +1,76 @@
+type WorktreeRename = {
+  oldWorktreeId: string
+  newWorktreeId: string
+}
+
+type WorktreeChangeEvent = {
+  repoId: string
+  renamed?: WorktreeRename
+}
+
+type WorktreeChangeRefreshHandler = (repoId: string, renamed?: WorktreeRename) => Promise<void>
+
+type QueuedWorktreeChange = {
+  renamed?: WorktreeRename
+}
+
+type RepoRefreshState = {
+  running: boolean
+  queue: QueuedWorktreeChange[]
+}
+
+export type WorktreeChangeRefreshQueue = {
+  enqueue: (event: WorktreeChangeEvent) => void
+}
+
+export function createWorktreeChangeRefreshQueue(
+  handler: WorktreeChangeRefreshHandler
+): WorktreeChangeRefreshQueue {
+  const states = new Map<string, RepoRefreshState>()
+
+  const drain = async (repoId: string, state: RepoRefreshState): Promise<void> => {
+    state.running = true
+    try {
+      while (state.queue.length > 0) {
+        const next = state.queue.shift()
+        try {
+          await handler(repoId, next?.renamed)
+        } catch (error) {
+          console.error('Failed to refresh changed worktrees:', error)
+        }
+      }
+    } finally {
+      state.running = false
+      if (state.queue.length === 0) {
+        states.delete(repoId)
+      } else {
+        void drain(repoId, state)
+      }
+    }
+  }
+
+  return {
+    enqueue(event) {
+      let state = states.get(event.repoId)
+      if (!state) {
+        state = { running: false, queue: [] }
+        states.set(event.repoId, state)
+      }
+
+      if (event.renamed) {
+        state.queue.push({ renamed: event.renamed })
+      } else {
+        const lastQueued = state.queue.at(-1)
+        // Why: Windows/OneDrive can emit a burst for one checkout change. Keep a
+        // trailing refresh, but do not fan out adjacent identical repo scans.
+        if (!lastQueued || lastQueued.renamed !== undefined) {
+          state.queue.push({})
+        }
+      }
+
+      if (!state.running) {
+        void drain(event.repoId, state)
+      }
+    }
+  }
+}