From 5b73cfcc985c059aaacecb07a77d602b6250fe28 Mon Sep 17 00:00:00 2001 From: Jason Chai <35755092+stwith@users.noreply.github.com> Date: Fri, 31 Jul 2026 18:18:58 +0800 Subject: [PATCH] fix(desktop): bound review workspace scans --- apps/desktop/electron/git-review-ops.test.ts | 32 +++++++++++++++++++- apps/desktop/electron/git-review-ops.ts | 27 +++++++++++++---- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/apps/desktop/electron/git-review-ops.test.ts b/apps/desktop/electron/git-review-ops.test.ts index 3d102c082de10..9880b53ab33c8 100644 --- a/apps/desktop/electron/git-review-ops.test.ts +++ b/apps/desktop/electron/git-review-ops.test.ts @@ -6,7 +6,7 @@ import path from 'node:path' import { afterEach, test } from 'vitest' -import { gitFor, repoStatus, resolveRenamePath } from './git-review-ops' +import { gitFor, repoStatus, resolveRenamePath, REVIEW_FILE_CAP, reviewList } from './git-review-ops' const tempDirs: string[] = [] @@ -87,3 +87,33 @@ test('repoStatus reports an untracked directory without recursively listing its ['generated/'] ) }) + +test('reviewList reports an untracked directory without recursively listing its contents', async () => { + const dir = makeRepo() + const nested = path.join(dir, 'browser-profile', 'Default', 'Cache') + + fs.mkdirSync(nested, { recursive: true }) + + for (let i = 0; i < 20; i++) { + fs.writeFileSync(path.join(nested, `cache-${i}.bin`), 'generated\n') + } + + const result = await reviewList(dir, 'uncommitted', null, 'git') + + assert.deepEqual( + result.files.map(file => file.path), + ['browser-profile/'] + ) +}) + +test('reviewList caps the file payload returned to the renderer', async () => { + const dir = makeRepo() + + for (let i = 0; i < REVIEW_FILE_CAP + 10; i++) { + fs.writeFileSync(path.join(dir, `untracked-${String(i).padStart(4, '0')}.txt`), 'generated\n') + } + + const result = await reviewList(dir, 'uncommitted', null, 'git') + + assert.equal(result.files.length, REVIEW_FILE_CAP) +}) diff --git a/apps/desktop/electron/git-review-ops.ts b/apps/desktop/electron/git-review-ops.ts index 5fcb41fd0a18a..a44be7d3fe60b 100644 --- a/apps/desktop/electron/git-review-ops.ts +++ b/apps/desktop/electron/git-review-ops.ts @@ -14,6 +14,7 @@ import { resolveRequestedPathForIpc } from './hardening' const COMMIT_CONTEXT_DIFF_MAX_CHARS = 120_000 const COMMIT_CONTEXT_UNTRACKED_MAX = 80 +const REVIEW_FILE_CAP = 2_000 const UNTRACKED_LINE_COUNT_CONCURRENCY = 16 const UNTRACKED_LINE_COUNT_MAX_BYTES = 1024 * 1024 @@ -253,7 +254,7 @@ async function reviewList(repoPath, scope, baseRef, gitBin) { const range = scope === 'branch' ? `${base}...HEAD` : base const summary = await git.diffSummary([range]) - const files = summary.files.map(file => ({ + const files = summary.files.slice(0, REVIEW_FILE_CAP).map(file => ({ path: resolveRenamePath(file.file), added: 'insertions' in file ? file.insertions : 0, removed: 'deletions' in file ? file.deletions : 0, @@ -262,12 +263,22 @@ async function reviewList(repoPath, scope, baseRef, gitBin) { })) // "Last turn" also surfaces files created since the baseline (untracked). - if (scope === 'lastTurn') { - const status = await git.status() + if (scope === 'lastTurn' && files.length < REVIEW_FILE_CAP) { + // Keep untracked directories compact. A recursive status can produce + // hundreds of thousands of rows for browser profiles, generated + // artifacts, or dependency trees before the response reaches the + // renderer. + const status = await git.status(['--untracked-files=normal']) + const knownPaths = new Set(files.map(file => file.path)) for (const path of status.not_added) { - if (!files.some(f => f.path === path)) { + if (files.length >= REVIEW_FILE_CAP) { + break + } + + if (!knownPaths.has(path)) { files.push({ path, added: 0, removed: 0, status: '?', staged: false }) + knownPaths.add(path) } } } @@ -280,7 +291,10 @@ async function reviewList(repoPath, scope, baseRef, gitBin) { // Default: uncommitted (staged + unstaged + untracked), one row per path. const [status, staged, unstaged] = await Promise.all([ - git.status(), + // `normal` reports an untracked directory as one row instead of walking + // every descendant. The result is also capped before per-file stat/read + // work and before crossing the Electron IPC boundary. + git.status(['--untracked-files=normal']), git.diffSummary(['--cached']), git.diffSummary([]) ]) @@ -288,7 +302,7 @@ async function reviewList(repoPath, scope, baseRef, gitBin) { const stagedCounts = countsByPath(staged) const unstagedCounts = countsByPath(unstaged) - const files = status.files.map(file => { + const files = status.files.slice(0, REVIEW_FILE_CAP).map(file => { const filePath = resolveRenamePath(file.path) const sc = stagedCounts.get(filePath) || { added: 0, removed: 0 } const uc = unstagedCounts.get(filePath) || { added: 0, removed: 0 } @@ -696,6 +710,7 @@ export { gitFor, repoStatus, resolveRenamePath, + REVIEW_FILE_CAP, reviewCommit, reviewCommitContext, reviewCreatePr,