Stop the Cleanup dirty sweep from flooding remote hosts

src/main/index.ts

@@ -36,6 +36,7 @@ import type { ServerTransport } from '../shared/transport/transport'
 import { detectRuntime } from './paths'
 import { fixPathFromLoginShell } from './path-fix'
 import { ChatStartGate } from './chat-start-gate'
+import { createSingleFlight } from './single-flight'
 import {
   knownClaudeConfigDirs,
   onShellEnvCaptured,
@@ -1973,8 +1974,17 @@ function registerIpcHandlers(): void {
     }
   )
 
+  // Identical concurrent asks share one `git status`. A client that re-sends
+  // the same sweep on every render (the 2.18 Cleanup modal did, once per
+  // streamed token) otherwise queues thousands of reads, and each spawn
+  // blocks the event loop long enough that every other request — tab
+  // switches included — waits minutes behind them.
+  // Keyed on priority too, so a bulk sweep never hands its queued answer
+  // to an interactive caller.
+  const dirtyFlight = createSingleFlight<boolean>()
   transport.onRequest('worktree:isDirty', async (_ctx, path: string, opts?: { bulk?: boolean }) => {
-    const git = await isWorktreeDirty(path, opts?.bulk ? 'bulk' : 'interactive')
+    const priority = opts?.bulk ? 'bulk' : 'interactive'
+    const git = await dirtyFlight(`${priority}\0${path}`, () => isWorktreeDirty(path, priority))
     const scratchpad = hasScratchpadNote(store.getSnapshot().state.scratchpad, path)
     return { git, scratchpad }
   })

src/main/single-flight.test.ts

@@ -0,0 +1,55 @@
+import { describe, it, expect, vi } from 'vitest'
+import { createSingleFlight } from './single-flight'
+
+function deferred<T>(): { promise: Promise<T>; resolve: (v: T) => void; reject: (e: unknown) => void } {
+  let resolve!: (v: T) => void
+  let reject!: (e: unknown) => void
+  const promise = new Promise<T>((res, rej) => {
+    resolve = res
+    reject = rej
+  })
+  return { promise, resolve, reject }
+}
+
+describe('createSingleFlight', () => {
+  it('shares one underlying call between concurrent callers with the same key', async () => {
+    const flight = createSingleFlight<boolean>()
+    const d = deferred<boolean>()
+    const fn = vi.fn(() => d.promise)
+
+    const calls = Array.from({ length: 50 }, () => flight('a', fn))
+    expect(fn).toHaveBeenCalledTimes(1)
+
+    d.resolve(true)
+    expect(await Promise.all(calls)).toEqual(Array(50).fill(true))
+  })
+
+  it('keeps different keys separate', () => {
+    const flight = createSingleFlight<boolean>()
+    const fn = vi.fn(() => new Promise<boolean>(() => {}))
+    void flight('a', fn)
+    void flight('b', fn)
+    expect(fn).toHaveBeenCalledTimes(2)
+  })
+
+  it('runs fresh after the shared call resolves', async () => {
+    const flight = createSingleFlight<number>()
+    let n = 0
+    const fn = vi.fn(async () => ++n)
+    expect(await flight('a', fn)).toBe(1)
+    expect(await flight('a', fn)).toBe(2)
+    expect(fn).toHaveBeenCalledTimes(2)
+  })
+
+  it('clears the entry on reject, so the next call retries', async () => {
+    const flight = createSingleFlight<number>()
+    const failing = vi.fn(() => Promise.reject(new Error('boom')))
+    const a = flight('a', failing)
+    const b = flight('a', failing)
+    await expect(a).rejects.toThrow('boom')
+    await expect(b).rejects.toThrow('boom')
+    expect(failing).toHaveBeenCalledTimes(1)
+
+    expect(await flight('a', async () => 7)).toBe(7)
+  })
+})

src/main/single-flight.ts

@@ -0,0 +1,14 @@
+/** Collapses concurrent calls with the same key onto one in-flight promise.
+ *  The entry is dropped as soon as that promise settles, either way, so a
+ *  call made afterwards always runs fresh — this dedupes a burst, it never
+ *  caches an answer. */
+export function createSingleFlight<T>(): (key: string, fn: () => Promise<T>) => Promise<T> {
+  const inflight = new Map<string, Promise<T>>()
+  return (key, fn) => {
+    const pending = inflight.get(key)
+    if (pending) return pending
+    const started = fn().finally(() => inflight.delete(key))
+    inflight.set(key, started)
+    return started
+  }
+}

src/renderer/App.tsx

@@ -1962,6 +1962,7 @@ const setQuestStep = useCallback((next: QuestStep) => {
               prStatuses={prStatuses}
               mergedPaths={mergedPaths}
               lastActive={lastActive}
+              backendFor={backendFor}
               onBulkDelete={handleBulkDeleteWorktrees}
             />
           </div>

src/renderer/components/Cleanup.tsx

@@ -1,6 +1,6 @@
-import { useEffect, useMemo, useState } from 'react'
+import { useEffect, useMemo, useRef, useState } from 'react'
 import { ArrowLeft, Trash2, AlertTriangle, GitPullRequest, CheckCircle2, Loader2, ChevronDown, ChevronRight, ExternalLink } from 'lucide-react'
-import type { Worktree, PRStatus, ActivityLog, BranchCommit } from '../types'
+import type { Worktree, PRStatus, ActivityLog, BranchCommit, ElectronAPI } from '../types'
 import { isPRMerged } from '../../shared/state/prs'
 import { useBackend } from '../backend'
 import { useAliases } from '../store'
@@ -11,6 +11,10 @@ interface CleanupProps {
   prStatuses: Record<string, PRStatus | null>
   mergedPaths: Record<string, boolean>
   lastActive: Record<string, number>
+  /** The backend hosting each worktree. The list spans every backend in
+   *  fleet view, and asking the active one about a remote path gets a
+   *  wrong answer from the wrong machine. */
+  backendFor: (worktreePath: string) => ElectronAPI
   onBulkDelete: (
     paths: string[],
     force: boolean,
@@ -64,6 +68,7 @@ export function Cleanup({
   prStatuses,
   mergedPaths,
   lastActive,
+  backendFor,
   onBulkDelete
 }: CleanupProps): JSX.Element {
   const backend = useBackend()
@@ -74,7 +79,8 @@ export function Cleanup({
   const [includeDirty, setIncludeDirty] = useState(false)
   // Absent key = not scanned yet. Distinct from `false` (scanned, clean).
   const [dirtyMap, setDirtyMap] = useState<Record<string, boolean>>({})
-  const [scanningDirty, setScanningDirty] = useState(true)
+  // Answered with an error: no longer pending, but still not known clean.
+  const [dirtyFailed, setDirtyFailed] = useState<Record<string, boolean>>({})
   const [activityLastTs, setActivityLastTs] = useState<Record<string, number>>({})
   const [selected, setSelected] = useState<Record<string, boolean>>({})
   // Paths the user has explicitly clicked; their selection state must survive
@@ -108,61 +114,83 @@ export function Cleanup({
   // A path missing from dirtyMap means "not scanned yet", which is distinct
   // from "clean" — see the selection defaults below, which must not offer up
   // an unscanned worktree for deletion.
+  //
+  // Each path is asked about once per open, never per `worktrees` change.
+  // The list legitimately changes while the modal is open — every deletion
+  // from it is one — and rescanning the whole set each time multiplied the
+  // sweep by the number of deletions. Requests can't be cancelled once sent,
+  // so a re-run only ever adds the paths it hasn't asked about yet.
+  const requestedDirty = useRef(new Set<string>())
+  const mounted = useRef(true)
+  useEffect(() => {
+    mounted.current = true
+    return () => {
+      mounted.current = false
+    }
+  }, [])
   useEffect(() => {
-    let cancelled = false
-    setScanningDirty(true)
-    let outstanding = eligible.length
-    if (outstanding === 0) setScanningDirty(false)
     for (const w of eligible) {
-      void backend
+      if (requestedDirty.current.has(w.path)) continue
+      requestedDirty.current.add(w.path)
+      void backendFor(w.path)
         .isWorktreeDirty(w.path, { bulk: true })
         .then((d) => {
-          if (cancelled) return
+          if (!mounted.current) return
           setDirtyMap((prev) => ({ ...prev, [w.path]: d.git || d.scratchpad }))
         })
-        .catch(() => {})
-        .finally(() => {
-          if (cancelled) return
-          if (--outstanding === 0) setScanningDirty(false)
+        .catch(() => {
+          if (!mounted.current) return
+          setDirtyFailed((prev) => ({ ...prev, [w.path]: true }))
         })
     }
-    return () => {
-      cancelled = true
-    }
-  }, [eligible, backend])
+  }, [eligible, backendFor])
+  const scanningDirty = eligible.some(
+    (w) => dirtyMap[w.path] === undefined && !dirtyFailed[w.path]
+  )
 
+  // Each backend keeps its own activity log, and in fleet view the list
+  // spans several, so the log is fetched once from every backend that owns
+  // an eligible worktree. Like the dirty sweep, a backend is asked once per
+  // open: a deletion changes `eligible` but not anyone's history.
+  // `backendFor` hands back one cached object per backend, so it is the key.
+  const requestedLogs = useRef(new Set<ElectronAPI>())
+  const logsInFlight = useRef(0)
   useEffect(() => {
-    let cancelled = false
-    ;(async () => {
-      setLoading(true)
-      try {
-        const log = await backend.getActivityLog()
-        if (cancelled) return
-        const lastTs: Record<string, number> = {}
-        for (const [path, rec] of Object.entries(log as ActivityLog)) {
-          // Only 'processing' proves the agent actually ran here. Waking a
-          // chat session derives 'waiting' and auto-sleep derives 'idle',
-          // so using the newest event of any kind resets a worktree's age
-          // just from the user opening it.
-          let ts: number | undefined
-          for (let i = rec.events.length - 1; i >= 0; i--) {
-            if (rec.events[i].s === 'processing') {
-              ts = rec.events[i].t
-              break
+    for (const w of eligible) {
+      const owner = backendFor(w.path)
+      if (requestedLogs.current.has(owner)) continue
+      requestedLogs.current.add(owner)
+      logsInFlight.current++
+      void owner
+        .getActivityLog()
+        .then((log) => {
+          if (!mounted.current) return
+          const lastTs: Record<string, number> = {}
+          for (const [path, rec] of Object.entries(log as ActivityLog)) {
+            if (backendFor(path) !== owner) continue
+            // Only 'processing' proves the agent actually ran here. Waking a
+            // chat session derives 'waiting' and auto-sleep derives 'idle',
+            // so using the newest event of any kind resets a worktree's age
+            // just from the user opening it.
+            let ts: number | undefined
+            for (let i = rec.events.length - 1; i >= 0; i--) {
+              if (rec.events[i].s === 'processing') {
+                ts = rec.events[i].t
+                break
+              }
             }
+            ts ??= rec.createdAt
+            if (ts !== undefined) lastTs[path] = ts
           }
-          ts ??= rec.createdAt
-          if (ts !== undefined) lastTs[path] = ts
-        }
-        setActivityLastTs(lastTs)
-      } finally {
-        if (!cancelled) setLoading(false)
-      }
-    })()
-    return () => {
-      cancelled = true
+          setActivityLastTs((prev) => ({ ...prev, ...lastTs }))
+        })
+        .catch(() => {})
+        .finally(() => {
+          if (--logsInFlight.current === 0 && mounted.current) setLoading(false)
+        })
     }
-  }, [eligible])
+    if (logsInFlight.current === 0) setLoading(false)
+  }, [eligible, backendFor])
 
   const ageMs = AGES.find((a) => a.id === ageKey)!.ms
 
@@ -240,7 +268,7 @@ export function Cleanup({
     })
     if (!commitsByPath[path]) {
       setCommitsByPath((prev) => ({ ...prev, [path]: 'loading' }))
-      backend
+      backendFor(path)
         .getBranchCommits(path)
         .then((commits) => {
           setCommitsByPath((prev) => ({ ...prev, [path]: commits }))

src/renderer/merge-backend-states.test.ts

@@ -163,4 +163,76 @@ describe('getMergedWorktrees (shared cache)', () => {
     expect(second.statuses.t1).toBe('processing')
     expect(second.tabsByWorktree['/w']).toBe(first.tabsByWorktree['/w']) // but not the tab walk
   })
+
+  it('keeps unchanged outputs by reference when another field changes', () => {
+    const list = [wt('/w', '/r')]
+    const first = getMergedWorktrees(
+      [backend('local', { kind: 'local', worktrees: list, statuses: { t1: 'idle' } })],
+      'merged',
+      'local'
+    )
+    const second = getMergedWorktrees(
+      [backend('local', { kind: 'local', worktrees: list, statuses: { t1: 'processing' } })],
+      'merged',
+      'local'
+    )
+    expect(second.statuses).not.toBe(first.statuses)
+    expect(second.flatWorktrees).toBe(first.flatWorktrees)
+    expect(second.worktrees).toBe(first.worktrees)
+    expect(second.pathBackendId).toBe(first.pathBackendId)
+    expect(second.backendMeta).toBe(first.backendMeta)
+  })
+
+  it('keeps every output stable in single mode when a hidden backend streams', () => {
+    // Single mode merges only the active backend, but the cache key spans
+    // all of them — a token on a hidden remote still reruns the merge.
+    const local = backend('local', {
+      kind: 'local',
+      worktrees: [wt('/l/a', '/l')],
+      repoRoots: ['/l'],
+      pending: [pending('p1', '/l')],
+      statuses: { t1: 'idle' }
+    })
+    const first = getMergedWorktrees(
+      [local, backend('beta', { worktrees: [wt('/b/a', '/b')], statuses: { b1: 'idle' } })],
+      'single',
+      'local'
+    )
+    const second = getMergedWorktrees(
+      [local, backend('beta', { worktrees: [wt('/b/a', '/b')], statuses: { b1: 'processing' } })],
+      'single',
+      'local'
+    )
+    expect(second).toBe(first)
+    for (const key of Object.keys(first) as (keyof typeof first)[]) {
+      expect(second[key], key).toBe(first[key])
+    }
+  })
+
+  it('returns the previous result when an event changed nothing it reads', () => {
+    const list = [wt('/w', '/r')]
+    const first = getMergedWorktrees(
+      [backend('local', { kind: 'local', worktrees: list })],
+      'merged',
+      'local'
+    )
+    const second = getMergedWorktrees(
+      [backend('local', { kind: 'local', worktrees: list })],
+      'merged',
+      'local'
+    )
+    expect(second).toBe(first)
+  })
+
+  it('hands out a new worktree list when a worktree is added', () => {
+    const a = wt('/a', '/r')
+    const first = getMergedWorktrees([backend('local', { kind: 'local', worktrees: [a] })], 'merged', 'local')
+    const second = getMergedWorktrees(
+      [backend('local', { kind: 'local', worktrees: [a, wt('/b', '/r')] })],
+      'merged',
+      'local'
+    )
+    expect(second.flatWorktrees).not.toBe(first.flatWorktrees)
+    expect(second.flatWorktrees.map((w) => w.path)).toEqual(['/a', '/b'])
+  })
 })