fix(source-control): keep huge change sets responsive (#9477)

* fix(source-control): keep huge change sets responsive

* Fix cancellation and retry handling for capped status

* Harden capped status for conflict-heavy repositories

* Harden capped status recovery and cancellation

* fix(source-control): preserve capped status correctness

* fix(source-control): translate submodule status at render time

---------

Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>

src/main/git/runner-command-exec.test.ts

@@ -661,6 +661,22 @@ describe('gitStreamStdout', () => {
     await rejection
     expect(child.kill).toHaveBeenCalled()
   })
+
+  it('handles a late spawn error after cancellation', async () => {
+    const child = createMockChildProcess(0)
+    spawnMock.mockReturnValue(child)
+    const controller = new AbortController()
+
+    const promise = gitStreamStdout(['status'], {
+      cwd: '/repo',
+      signal: controller.signal,
+      onStdout: () => {}
+    })
+    controller.abort()
+
+    await expect(promise).rejects.toMatchObject({ name: 'AbortError' })
+    expect(() => child.emit('error', new Error('spawn ENOENT'))).not.toThrow()
+  })
 })
 
 describe('translateWslOutputPaths', () => {

src/main/git/runner.ts

@@ -1035,6 +1035,10 @@ export async function gitStreamStdout(
         finish(new Error(`git exited with ${code}: ${stderr}`))
       }
       function onAbort(): void {
+        if (!child.pid) {
+          // Why: failed spawn reports ENOENT after abort cleanup; retain a listener so it cannot crash main.
+          child.once('error', () => {})
+        }
         void killSpawnedCommandTree(child)
         finish(createAbortError())
       }

src/main/git/status-porcelain-parser.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from 'vitest'
-import { StatusPorcelainParser } from './status-porcelain-parser'
+import { StatusPorcelainParser } from '../../shared/git-status-porcelain-parser'
 
 describe('StatusPorcelainParser', () => {
   it('parses branch headers and changed/untracked/ignored records', () => {
@@ -64,6 +64,7 @@ describe('StatusPorcelainParser', () => {
     parser.finish()
     expect(parser.entries).toEqual([])
     expect(parser.unmergedLines).toHaveLength(1)
+    expect(parser.statusLength).toBe(1)
   })
 
   it('carries a partial trailing line across chunk boundaries', () => {
@@ -94,6 +95,41 @@ describe('StatusPorcelainParser', () => {
     expect(parser.statusLength).toBe(4)
   })
 
+  it('counts unmerged records toward the stop limit', () => {
+    const parser = new StatusPorcelainParser()
+    const line = 'u UU N... 100644 100644 100644 100644 aa bb cc conflicted.ts\n'
+    const stopped = parser.update(line.repeat(4), 3)
+
+    expect(stopped).toBe(true)
+    expect(parser.unmergedLines).toHaveLength(4)
+    expect(parser.statusLength).toBe(4)
+  })
+
+  it('preserves deferred conflicts in status output order', () => {
+    const parser = new StatusPorcelainParser()
+    parser.update(
+      '? before.ts\n' +
+        'u UU N... 100644 100644 100644 100644 aa bb cc conflict.ts\n' +
+        '? after.ts\n',
+      0
+    )
+
+    expect(parser.statusRecords).toEqual([
+      {
+        type: 'entry',
+        entry: { path: 'before.ts', status: 'untracked', area: 'untracked' }
+      },
+      {
+        type: 'unmerged',
+        line: 'u UU N... 100644 100644 100644 100644 aa bb cc conflict.ts'
+      },
+      {
+        type: 'entry',
+        entry: { path: 'after.ts', status: 'untracked', area: 'untracked' }
+      }
+    ])
+  })
+
   it('does not signal stop when limit is 0 (disabled)', () => {
     const parser = new StatusPorcelainParser()
     const lines = `${Array.from({ length: 50 }, (_, i) => `? f${i}.txt`).join('\n')}\n`

src/main/git/status-porcelain-parser.ts

@@ -1,228 +0,0 @@
-import type { GitStatusEntry } from '../../shared/git-status-types'
-import { decodeGitCQuotedPath } from '../../shared/git-cquoted-path'
-
-/**
- * Incremental parser for `git status --porcelain=v2 --branch` output.
- *
- * Why incremental: a repo with an enormous un-ignored folder can emit a status
- * listing too large to buffer into one string (it overflows V8's max string
- * length and crashes the process). Feeding chunks here as they arrive lets the
- * caller stop git the moment the changed-entry count crosses a limit, so memory
- * stays bounded. Records are newline-delimited; a partial trailing line is
- * carried across chunks.
- *
- * Sync record types (1/2/?/!) are parsed into `entries`/`ignoredPaths` here.
- * Unmerged (`u`) records need async per-file git lookups, so their raw lines are
- * collected and resolved by the caller after the stream ends — they signal
- * conflict states and are never the source of huge output.
- */
-export type BranchMetadata = {
-  head?: string
-  branch?: string
-  upstreamName?: string
-  upstreamAheadBehind?: { ahead: number; behind: number }
-}
-
-export class StatusPorcelainParser {
-  private carry = ''
-  /** Count of changed-file entries seen — the limit is measured against this. */
-  private count = 0
-
-  readonly entries: GitStatusEntry[] = []
-  readonly ignoredPaths: string[] = []
-  /** Raw `u ` lines for the caller to resolve asynchronously. */
-  readonly unmergedLines: string[] = []
-  readonly branch: BranchMetadata = {}
-
-  /** Total changed-file entries observed (including any past the limit). */
-  get statusLength(): number {
-    return this.count
-  }
-
-  /**
-   * Feed one decoded chunk. Returns true once the accumulated changed-entry
-   * count exceeds `limit` (limit 0 disables the cap), signaling the caller to
-   * stop git. Complete lines are parsed; an incomplete trailing line is carried.
-   */
-  update(chunk: string, limit: number): boolean {
-    const text = this.carry + chunk
-    let start = 0
-    while (true) {
-      const nl = text.indexOf('\n', start)
-      if (nl === -1) {
-        break
-      }
-      // Strip a trailing \r so Windows CRLF output parses cleanly.
-      let end = nl
-      if (end > start && text.charCodeAt(end - 1) === 13) {
-        end -= 1
-      }
-      this.parseLine(text.slice(start, end))
-      start = nl + 1
-      if (limit !== 0 && this.count > limit) {
-        this.carry = ''
-        return true
-      }
-    }
-    this.carry = text.slice(start)
-    return false
-  }
-
-  /** Flush a final line with no trailing newline (e.g. when git exits). */
-  finish(): void {
-    if (this.carry.length > 0) {
-      this.parseLine(this.carry)
-      this.carry = ''
-    }
-  }
-
-  private parseLine(line: string): void {
-    if (!line) {
-      return
-    }
-    if (line.startsWith('# branch.oid ')) {
-      this.branch.head = line.slice('# branch.oid '.length).trim()
-      return
-    }
-    if (line.startsWith('# branch.head ')) {
-      const branchHead = line.slice('# branch.head '.length).trim()
-      // Why: undefined (not '') keeps this transport-compatible — the renderer
-      // turns "head without branch" into an explicit detached-HEAD clear.
-      this.branch.branch =
-        branchHead && branchHead !== '(detached)' ? `refs/heads/${branchHead}` : undefined
-      return
-    }
-    if (line.startsWith('# branch.upstream ')) {
-      this.branch.upstreamName = line.slice('# branch.upstream '.length).trim() || undefined
-      return
-    }
-    if (line.startsWith('# branch.ab ')) {
-      const match = line.match(/^# branch\.ab \+(\d+) -(\d+)$/)
-      if (match) {
-        this.branch.upstreamAheadBehind = {
-          ahead: Number.parseInt(match[1], 10),
-          behind: Number.parseInt(match[2], 10)
-        }
-      }
-      return
-    }
-    if (line.startsWith('1 ') || line.startsWith('2 ')) {
-      this.parseChangedEntry(line)
-      return
-    }
-    if (line.startsWith('? ')) {
-      this.push({
-        path: decodeGitCQuotedPath(line.slice(2)),
-        status: 'untracked',
-        area: 'untracked'
-      })
-      return
-    }
-    if (line.startsWith('! ')) {
-      this.ignoredPaths.push(decodeGitCQuotedPath(line.slice(2)))
-      return
-    }
-    if (line.startsWith('u ')) {
-      this.unmergedLines.push(line)
-    }
-  }
-
-  private parseChangedEntry(line: string): void {
-    // Changed entries: "1 XY sub mH mI mW hH path" or
-    // "2 XY sub mH mI mW hH X<score> path\torigPath"
-    const parts = line.split(' ')
-    const xy = parts[1]
-    const indexStatus = xy[0]
-    const worktreeStatus = xy[1]
-
-    if (line.startsWith('2 ')) {
-      // Why: porcelain v2 type-2 records put the new path after 9 fixed
-      // space-delimited fields and the old path after the tab. Preserving spaces
-      // keeps row actions and numstat counts keyed correctly.
-      const tabParts = line.split('\t')
-      const path = decodeGitCQuotedPath(tabParts[0].split(' ').slice(9).join(' '))
-      const oldPath = decodeGitCQuotedPath(tabParts.slice(1).join('\t'))
-      if (indexStatus !== '.') {
-        this.push({
-          path,
-          status: parseStatusChar(indexStatus),
-          area: 'staged',
-          oldPath,
-          ...submoduleStatusField(parts[2], indexStatus)
-        })
-      }
-      if (worktreeStatus !== '.') {
-        this.push({
-          path,
-          status: parseStatusChar(worktreeStatus),
-          area: 'unstaged',
-          oldPath,
-          ...submoduleStatusField(parts[2], worktreeStatus)
-        })
-      }
-      return
-    }
-
-    const path = decodeGitCQuotedPath(parts.slice(8).join(' '))
-    if (indexStatus !== '.') {
-      this.push({
-        path,
-        status: parseStatusChar(indexStatus),
-        area: 'staged',
-        ...submoduleStatusField(parts[2], indexStatus)
-      })
-    }
-    if (worktreeStatus !== '.') {
-      this.push({
-        path,
-        status: parseStatusChar(worktreeStatus),
-        area: 'unstaged',
-        ...submoduleStatusField(parts[2], worktreeStatus)
-      })
-    }
-  }
-
-  private push(entry: GitStatusEntry): void {
-    this.count += 1
-    this.entries.push(entry)
-  }
-}
-
-export function parseStatusChar(char: string): GitStatusEntry['status'] {
-  switch (char) {
-    case 'M':
-      return 'modified'
-    case 'A':
-      return 'added'
-    case 'D':
-      return 'deleted'
-    case 'R':
-      return 'renamed'
-    case 'C':
-      return 'copied'
-    default:
-      return 'modified'
-  }
-}
-
-export function parseSubmoduleStatus(
-  submoduleField: string | undefined,
-  statusChar = '.'
-): GitStatusEntry['submodule'] {
-  if (!submoduleField?.startsWith('S')) {
-    return undefined
-  }
-  return {
-    commitChanged: submoduleField[1] === 'C' || (submoduleField === 'S...' && statusChar === 'M'),
-    trackedChanges: submoduleField[2] === 'M',
-    untrackedChanges: submoduleField[3] === 'U'
-  }
-}
-
-function submoduleStatusField(
-  submoduleField: string | undefined,
-  statusChar: string
-): { submodule: GitStatusEntry['submodule'] } | {} {
-  const submodule = parseSubmoduleStatus(submoduleField, statusChar)
-  return submodule ? { submodule } : {}
-}

src/main/git/status.test.ts

@@ -1040,6 +1040,34 @@ describe('getSubmoduleStatus', () => {
     expect(result.entries).toContainEqual(
       expect.objectContaining({ path: 'lib/main.dart', status: 'modified', area: 'unstaged' })
     )
+    expect(gitExecFileAsyncMock.mock.calls.some(([args]) => args.includes('status'))).toBe(false)
+  })
+
+  it('caps staged commit-range entries before returning them to the renderer', async () => {
+    const OLD_OID = 'a'.repeat(40)
+    const NEW_OID = 'b'.repeat(40)
+    gitExecFileAsyncMock.mockReset()
+    gitExecFileAsyncMock.mockImplementation((args: string[]) => {
+      if (args.includes('--name-status')) {
+        return Promise.resolve({ stdout: 'M\tlib/a.dart\nM\tlib/b.dart\n' })
+      }
+      if (args[0] === 'ls-files') {
+        return Promise.resolve({ stdout: `160000 ${NEW_OID} 0\tflutter_mine\n` })
+      }
+      if (args[0] === 'ls-tree') {
+        return Promise.resolve({ stdout: `160000 commit ${OLD_OID}\tflutter_mine\n` })
+      }
+      return Promise.resolve({ stdout: '' })
+    })
+
+    const result = await getSubmoduleStatus('/repo', 'flutter_mine', {
+      staged: true,
+      limit: 1
+    })
+
+    expect(result.entries).toHaveLength(1)
+    expect(result.didHitLimit).toBe(true)
+    expect(result.statusLength).toBe(2)
   })
 })
 
@@ -1685,6 +1713,50 @@ describe('getStatus', () => {
     expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1)
   })
 
+  it('caps unmerged conflicts and keeps the visible conflict rows', async () => {
+    readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n')
+    existsSyncMock.mockReturnValue(true)
+    const lines = [
+      'u UU S... 160000 160000 160000 160000 aa bb cc vendor/submodule',
+      ...Array.from(
+        { length: 3 },
+        (_, i) => `u UU N... 100644 100644 100644 100644 aa bb cc conflict-${i}.ts`
+      )
+    ].join('\n')
+    gitExecFileAsyncMock.mockReset()
+    gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: `${lines}\n` })
+
+    const result = await getStatus('/repo', { limit: 2 })
+
+    expect(result.didHitLimit).toBe(true)
+    expect(result.statusLength).toBe(3)
+    expect(result.entries).toHaveLength(2)
+    expect(result.entries.map((entry) => entry.path)).toEqual(['conflict-0.ts', 'conflict-1.ts'])
+    expect(result.entries.every((entry) => entry.conflictStatus === 'unresolved')).toBe(true)
+    expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1)
+  })
+
+  it('keeps an early conflict ahead of later ordinary rows at the cap', async () => {
+    readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n')
+    existsSyncMock.mockReturnValue(true)
+    const lines = [
+      '? before.ts',
+      'u UU N... 100644 100644 100644 100644 aa bb cc conflict.ts',
+      '? after.ts'
+    ].join('\n')
+    gitExecFileAsyncMock.mockReset()
+    gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: `${lines}\n` })
+
+    const result = await getStatus('/repo', { limit: 2 })
+
+    expect(result.didHitLimit).toBe(true)
+    expect(result.entries.map((entry) => entry.path)).toEqual(['before.ts', 'conflict.ts'])
+    expect(result.entries[1]).toMatchObject({
+      conflictKind: 'both_modified',
+      conflictStatus: 'unresolved'
+    })
+  })
+
   it('does not flag didHitLimit for a normal repo under the limit', async () => {
     readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n')
     existsSyncMock.mockReturnValue(false)

src/main/git/status.ts

@@ -37,8 +37,8 @@ import {
   gitOptionalLocksDisabledEnv,
   gitStreamStdout
 } from './runner'
-import { StatusPorcelainParser } from './status-porcelain-parser'
-import { DEFAULT_GIT_STATUS_LIMIT } from '../../shared/git-status-limit'
+import { StatusPorcelainParser } from '../../shared/git-status-porcelain-parser'
+import { capGitStatusEntries, resolveGitStatusLimit } from '../../shared/git-status-limit'
 import { describeMaxBufferOverflowError, isMaxBufferOverflowError } from './max-buffer-overflow'
 import {
   removeSafeUntrackedDiscardTarget,
@@ -231,10 +231,7 @@ export async function getStatus(
 
 function getStatusReadKey(worktreePath: string, options: GetStatusOptions): string {
   // Why: each key part can change the output shape or runtime routing.
-  const limit =
-    typeof options.limit === 'number' && Number.isInteger(options.limit) && options.limit >= 0
-      ? options.limit
-      : DEFAULT_GIT_STATUS_LIMIT
+  const limit = resolveGitStatusLimit(options.limit)
   return [
     worktreePath,
     options.wslDistro ?? '',
@@ -254,10 +251,7 @@ async function runGetStatus(
   let effectiveUpstreamStatus: GitUpstreamStatus | undefined
   let statusSucceeded = false
   // Why: a bad limit (negative/fractional/NaN) breaks early-stop; require a valid non-negative int (0 disables the cap).
-  const limit =
-    typeof options.limit === 'number' && Number.isInteger(options.limit) && options.limit >= 0
-      ? options.limit
-      : DEFAULT_GIT_STATUS_LIMIT
+  const limit = resolveGitStatusLimit(options.limit)
 
   // Why: detectConflictOperation and git status are independent, so run them concurrently to save I/O latency.
   const conflictPromise = detectConflictOperation(worktreePath)
@@ -301,14 +295,19 @@ async function runGetStatus(
     // Not a git repo or git not available
   }
 
-  // Why: the parser overshoots by one (checks after pushing), so trim to exactly `limit`.
-  const entries = didHitLimit ? parser.entries.slice(0, limit) : parser.entries
+  const entries: GitStatusEntry[] = []
   const { head, branch, upstreamName, upstreamAheadBehind } = parser.branch
 
-  // Why: unmerged (`u`) records need async per-file lookups; resolve them off the hot path (conflicts are rare).
-  if (!didHitLimit) {
-    for (const line of parser.unmergedLines) {
-      const unmergedEntry = await parseUnmergedEntry(worktreePath, line)
+  // Why: resolve deferred conflicts in Git's output order so the cap cannot hide
+  // an early conflict behind ordinary rows that appeared later in the stream.
+  for (const record of parser.statusRecords) {
+    if (didHitLimit && entries.length >= limit) {
+      break
+    }
+    if (record.type === 'entry') {
+      entries.push(record.entry)
+    } else {
+      const unmergedEntry = await parseUnmergedEntry(worktreePath, record.line)
       if (unmergedEntry) {
         entries.push(unmergedEntry)
       }
@@ -414,10 +413,14 @@ export function resolveSubmoduleWorktreePath(worktreePath: string, submodulePath
 export async function getSubmoduleStatus(
   worktreePath: string,
   submodulePath: string,
-  options: GitRuntimeOptions & { staged?: boolean } = {}
+  options: GetStatusOptions & { staged?: boolean } = {}
 ): Promise<GitStatusResult> {
   const submoduleWorktreePath = resolveSubmoduleWorktreePath(worktreePath, submodulePath)
-  const workingResult = await getStatus(submoduleWorktreePath, options)
+  const limit = resolveGitStatusLimit(options.limit)
+  // Why: staged expansion only represents HEAD→index; scanning the submodule worktree is wasted work.
+  const workingResult = options.staged
+    ? ({ entries: [], conflictOperation: 'unknown' } satisfies GitStatusResult)
+    : await getStatus(submoduleWorktreePath, options)
   // Why: a moved gitlink (clean worktree) has no status rows; surface the parent-commit→checkout range as inner rows.
   const fromOid = options.staged
     ? await readGitlinkOidFromTree(worktreePath, 'HEAD', submodulePath, options)
@@ -434,7 +437,7 @@ export async function getSubmoduleStatus(
       options
     )
     if (options.staged) {
-      return { ...workingResult, entries: rangeEntries }
+      return { ...workingResult, ...capGitStatusEntries(rangeEntries, limit) }
     }
     const rangePaths = new Set(rangeEntries.map((entry) => entry.path))
     // Range rows win on overlap so the diff matches getDiff's commit-range route.
@@ -442,7 +445,10 @@ export async function getSubmoduleStatus(
       ...rangeEntries,
       ...workingResult.entries.filter((entry) => !rangePaths.has(entry.path))
     ]
-    return { ...workingResult, entries }
+    return {
+      ...workingResult,
+      ...capGitStatusEntries(entries, limit, workingResult)
+    }
   }
   if (options.staged) {
     return { ...workingResult, entries: [] }