fix(desktop): serialize git status refreshes (#65341)

apps/desktop/electron/git-review-ops.test.ts

@@ -1,8 +1,34 @@
 import assert from 'node:assert/strict'
+import { execFileSync } from 'node:child_process'
+import fs from 'node:fs'
+import os from 'node:os'
+import path from 'node:path'
 
-import { test } from 'vitest'
+import { afterEach, test } from 'vitest'
 
-import { resolveRenamePath } from './git-review-ops'
+import { repoStatus, resolveRenamePath } from './git-review-ops'
+
+const tempDirs: string[] = []
+
+afterEach(() => {
+  for (const dir of tempDirs.splice(0)) {
+    fs.rmSync(dir, { force: true, recursive: true })
+  }
+})
+
+function makeRepo() {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-desktop-git-status-'))
+
+  tempDirs.push(dir)
+  execFileSync('git', ['init', '-q'], { cwd: dir })
+  execFileSync('git', ['config', 'user.email', 'hermes-test@example.com'], { cwd: dir })
+  execFileSync('git', ['config', 'user.name', 'Hermes Test'], { cwd: dir })
+  fs.writeFileSync(path.join(dir, 'tracked.txt'), 'tracked\n')
+  execFileSync('git', ['add', 'tracked.txt'], { cwd: dir })
+  execFileSync('git', ['commit', '-qm', 'initial'], { cwd: dir })
+
+  return dir
+}
 
 test('resolveRenamePath: plain path is unchanged', () => {
   assert.equal(resolveRenamePath('src/a.ts'), 'src/a.ts')
@@ -19,3 +45,21 @@ test('resolveRenamePath: brace rename resolves to the new path', () => {
 test('resolveRenamePath: brace rename collapsing a segment', () => {
   assert.equal(resolveRenamePath('src/{lib => }/file.ts'), 'src/file.ts')
 })
+
+test('repoStatus reports an untracked directory without recursively listing its contents', async () => {
+  const dir = makeRepo()
+  const nested = path.join(dir, 'generated', 'deep')
+
+  fs.mkdirSync(nested, { recursive: true })
+  fs.writeFileSync(path.join(nested, 'large-output.txt'), 'generated\n')
+
+  const status = await repoStatus(dir, 'git')
+
+  assert.ok(status)
+  assert.equal(status.untracked, 1)
+  assert.equal(status.changed, 1)
+  assert.deepEqual(
+    status.files.map(file => file.path),
+    ['generated/']
+  )
+})

apps/desktop/electron/git-review-ops.ts

@@ -610,7 +610,11 @@ async function repoStatus(repoPath, gitBin) {
   let status
 
   try {
-    status = await git.status()
+    // The coding rail needs compact change truth, not every generated file.
+    // `simple-git` defaults bare `-u` to recursive `all`, which can make a
+    // generated workspace consume gigabytes before the 200-row UI cap is
+    // applied. `normal` reports each untracked directory as one entry.
+    status = await git.status(['--untracked-files=normal'])
   } catch {
     // Not a repo / git unavailable / remote backend.
     return null
@@ -652,11 +656,10 @@ async function repoStatus(repoPath, gitBin) {
   }
 
   // `git diff HEAD` ignores untracked files, so a turn that only creates new
-  // files (the common case — a fresh module, a demo dir) showed +0 in the rail
-  // while the review pane counted them. Fold untracked insertions into `added`
-  // so the rail matches reality. Bounded (size cap + concurrency) like the
-  // review tree; only the capped file slice is counted so a huge untracked tree
-  // can't stall the probe.
+  // files (the common case — a fresh module) showed +0 in the rail while the
+  // review pane counted them. Fold top-level untracked file insertions into
+  // `added`; directories reported by the compact `normal` scan intentionally
+  // remain at zero rather than recursively walking their contents.
   try {
     const untracked = status.not_added.slice(0, 500)
 

apps/desktop/src/store/coding-status.test.ts

@@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
 
 import type { HermesRepoStatus } from '@/global'
 
-import { $repoStatus, refreshRepoStatus } from './coding-status'
+import { $repoStatus, $repoStatusLoading, refreshRepoStatus } from './coding-status'
 import { $currentCwd } from './session'
 
 const sampleStatus: HermesRepoStatus = {
@@ -27,12 +27,15 @@ function stubProbe(impl: (cwd: string) => Promise<HermesRepoStatus | null>) {
 
 describe('refreshRepoStatus', () => {
   beforeEach(() => {
+    vi.useFakeTimers()
     $repoStatus.set(null)
     $currentCwd.set('')
     delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop
   })
 
   afterEach(() => {
+    vi.clearAllTimers()
+    vi.useRealTimers()
     delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop
   })
 
@@ -71,4 +74,47 @@ describe('refreshRepoStatus', () => {
     await refreshRepoStatus('/repo')
     expect($repoStatus.get()).toBeNull()
   })
+
+  it('runs one probe at a time and coalesces overlap into one trailing refresh', async () => {
+    const resolvers: Array<(status: HermesRepoStatus | null) => void> = []
+    const calls: string[] = []
+    let active = 0
+    let maxActive = 0
+
+    stubProbe(
+      cwd =>
+        new Promise(resolve => {
+          calls.push(cwd)
+          active++
+          maxActive = Math.max(maxActive, active)
+          resolvers.push(status => {
+            active--
+            resolve(status)
+          })
+        })
+    )
+
+    const first = refreshRepoStatus('/repo-a')
+    const second = refreshRepoStatus('/repo-b')
+    const third = refreshRepoStatus('/repo-c')
+
+    expect(calls).toEqual(['/repo-a'])
+    expect(maxActive).toBe(1)
+    expect($repoStatusLoading.get()).toBe(true)
+
+    resolvers.shift()?.(sampleStatus)
+    await Promise.resolve()
+    await Promise.resolve()
+
+    expect(calls).toEqual(['/repo-a', '/repo-c'])
+    expect(maxActive).toBe(1)
+    expect($repoStatus.get()).toBeNull()
+
+    resolvers.shift()?.(sampleStatus)
+    await Promise.all([first, second, third])
+
+    expect(maxActive).toBe(1)
+    expect($repoStatus.get()).toEqual(sampleStatus)
+    expect($repoStatusLoading.get()).toBe(false)
+  })
 })

apps/desktop/src/store/coding-status.ts

@@ -66,9 +66,19 @@ async function loadWorktrees(target: string): Promise<void> {
   }
 }
 
+interface RepoStatusRefreshRequest {
+  probe: (cwd: string) => Promise<HermesRepoStatus | null>
+  seq: number
+  target: string
+}
+
 // Coalesce overlapping probes: many triggers can fire around a turn boundary
-// (busy flip + worktree token + focus), but only the latest cwd matters.
+// (busy flip + worktree token + focus), but only the latest cwd matters. Keep
+// one probe in flight and retain at most one trailing request so a slow Git
+// status cannot multiply into an unbounded subprocess pile-up.
 let inflightCwd: null | string = null
+let pendingRepoStatusRefresh: RepoStatusRefreshRequest | null = null
+let repoStatusRefreshInFlight: Promise<void> | null = null
 let repoStatusRefreshSeq = 0
 let repoStatusRefreshTimer: ReturnType<typeof setTimeout> | null = null
 
@@ -79,21 +89,7 @@ const normalizeCwd = (cwd?: null | string): null | string => cwd?.trim() || null
  * Best-effort: a non-repo, a remote backend, or a missing probe clears the
  * status so the rail hides rather than showing stale data.
  */
-export async function refreshRepoStatus(cwd?: null | string): Promise<void> {
-  const target = normalizeCwd(cwd ?? $currentCwd.get())
-  const probe = desktopGit()?.repoStatus
-  const seq = (repoStatusRefreshSeq += 1)
-
-  if (!target || !probe) {
-    $repoStatus.set(null)
-    $repoWorktrees.set([])
-
-    return
-  }
-
-  inflightCwd = target
-  $repoStatusLoading.set(true)
-
+async function runRepoStatusRefresh({ probe, seq, target }: RepoStatusRefreshRequest): Promise<void> {
   try {
     const status = await probe(target)
 
@@ -114,13 +110,50 @@ export async function refreshRepoStatus(cwd?: null | string): Promise<void> {
       $repoStatus.set(null)
       $repoWorktrees.set([])
     }
-  } finally {
-    if (seq === repoStatusRefreshSeq && inflightCwd === target) {
-      $repoStatusLoading.set(false)
-    }
   }
 }
 
+async function drainRepoStatusRefreshes(): Promise<void> {
+  while (pendingRepoStatusRefresh) {
+    const request = pendingRepoStatusRefresh
+
+    pendingRepoStatusRefresh = null
+    await runRepoStatusRefresh(request)
+  }
+
+  // This reset is synchronous with the final empty-queue check. A refresh
+  // arriving before this continuation runs is drained above; one arriving
+  // afterward sees no in-flight promise and starts a new drain.
+  repoStatusRefreshInFlight = null
+  $repoStatusLoading.set(false)
+}
+
+export function refreshRepoStatus(cwd?: null | string): Promise<void> {
+  const target = normalizeCwd(cwd ?? $currentCwd.get())
+  const probe = desktopGit()?.repoStatus
+  const seq = (repoStatusRefreshSeq += 1)
+
+  if (!target || !probe) {
+    pendingRepoStatusRefresh = null
+    inflightCwd = null
+    $repoStatus.set(null)
+    $repoWorktrees.set([])
+    $repoStatusLoading.set(false)
+
+    return repoStatusRefreshInFlight || Promise.resolve()
+  }
+
+  inflightCwd = target
+  pendingRepoStatusRefresh = { probe, seq, target }
+  $repoStatusLoading.set(true)
+
+  if (!repoStatusRefreshInFlight) {
+    repoStatusRefreshInFlight = drainRepoStatusRefreshes()
+  }
+
+  return repoStatusRefreshInFlight
+}
+
 function scheduleRepoStatusRefresh(cwd?: null | string): void {
   if (repoStatusRefreshTimer) {
     clearTimeout(repoStatusRefreshTimer)