From faa759d6068699804c7fd3b7d559e641d5dbebe6 Mon Sep 17 00:00:00 2001 From: Pranav Tomar Date: Sat, 14 Nov 2026 07:35:01 +0530 Subject: [PATCH] fix(worktrees): stream the submodule probe so large repos can be deleted (#42174) The delete preflight listed the whole index with a buffered `git ls-files --stage -z`, which overflows the 10 MiB Git output cap in large repositories and blocks workspace deletion. Stream the listing through gitStreamStdout and keep only gitlink (mode 160000) paths, so memory scales with the number of submodules instead of the index size. Fixes #42134 --- .../remove-worktree-clean-preflight.test.ts | 17 +++++--- .../removal/worktree-removal-preflight.ts | 43 +++++++++++++++---- .../removal/worktree-removal-real-git.test.ts | 26 +++++++++++ 3 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/main/git/worktrees/removal/remove-worktree-clean-preflight.test.ts b/src/main/git/worktrees/removal/remove-worktree-clean-preflight.test.ts index b891958bae4..1b454e6564d 100644 --- a/src/main/git/worktrees/removal/remove-worktree-clean-preflight.test.ts +++ b/src/main/git/worktrees/removal/remove-worktree-clean-preflight.test.ts @@ -7,14 +7,16 @@ const { translateWslOutputPathsMock, statMock, readFileMock, - resolveGitDirMock + resolveGitDirMock, + gitStreamStdoutMock } = vi.hoisted(() => ({ gitExecFileAsyncMock: vi.fn(), gitExecFileSyncMock: vi.fn(), translateWslOutputPathsMock: vi.fn((output: string) => output), statMock: vi.fn(), readFileMock: vi.fn(), - resolveGitDirMock: vi.fn() + resolveGitDirMock: vi.fn(), + gitStreamStdoutMock: vi.fn() })) vi.mock('../../command-runner/git-exec-file', async (importOriginal) => ({ @@ -22,6 +24,10 @@ vi.mock('../../command-runner/git-exec-file', async (importOriginal) => ({ gitExecFileAsync: gitExecFileAsyncMock })) +vi.mock('../../command-runner/git-stream-stdout', () => ({ + gitStreamStdout: gitStreamStdoutMock +})) + vi.mock('../../command-runner/git-exec-file-sync', async (importOriginal) => ({ ...(await importOriginal>()), gitExecFileSync: gitExecFileSyncMock @@ -56,10 +62,9 @@ beforeEach(() => { describe('assertWorktreeCleanForRemoval', () => { beforeEach(() => { - // Calls after the status read are the submodule probe's `ls-files`; failing leaves the verdict to Git. - gitExecFileAsyncMock - .mockReset() - .mockRejectedValue(new Error('git ls-files exited with code 128')) + gitExecFileAsyncMock.mockReset() + // A failed submodule probe leaves the verdict to Git. + gitStreamStdoutMock.mockReset().mockRejectedValue(new Error('git exited with 128: ')) }) it('ignores configured linked paths without mutating them before preflight', async () => { diff --git a/src/main/git/worktrees/removal/worktree-removal-preflight.ts b/src/main/git/worktrees/removal/worktree-removal-preflight.ts index dc6af37e7bb..65fff520b74 100644 --- a/src/main/git/worktrees/removal/worktree-removal-preflight.ts +++ b/src/main/git/worktrees/removal/worktree-removal-preflight.ts @@ -6,6 +6,7 @@ import { SUBMODULE_REMOVAL_REFUSAL } from '../../../../shared/worktree/removal/removal' import { gitExecFileAsync } from '../../command-runner/git-exec-file' +import { gitStreamStdout } from '../../command-runner/git-stream-stdout' import { resolveGitDir } from '../../source-control/resolve-git-dir' import type { WorktreeRemovalPreflightOptions } from '../worktree-operation-options' import { @@ -92,20 +93,46 @@ async function containsSubmoduleCheckout( if (adminDirectory && !(await isWorktreePathMissing(join(adminDirectory, 'modules')))) { return true } - const { stdout: index } = await gitExecFileAsync(['ls-files', '--stage', '-z'], execution) - for (const entry of index.split('\0')) { - if ( - entry.startsWith('160000 ') && - !(await isWorktreePathMissing( - join(hostWorktreePath, entry.slice(entry.indexOf('\t') + 1), '.git') - )) - ) { + for (const gitlink of await readGitlinkPaths(execution)) { + if (!(await isWorktreePathMissing(join(hostWorktreePath, gitlink, '.git')))) { return true } } return false } +/** Stream the index keeping only gitlinks; a large repository's full listing exceeds the buffered cap. */ +async function readGitlinkPaths( + execution: Parameters[1] +): Promise { + const { cwd, env, wslDistro, signal, admissionTier, timeout } = execution + const gitlinks: string[] = [] + const keepGitlink = (entry: string): void => { + if (entry.startsWith('160000 ')) { + gitlinks.push(entry.slice(entry.indexOf('\t') + 1)) + } + } + let carry = '' + await gitStreamStdout(['ls-files', '--stage', '-z'], { + cwd, + env, + wslDistro, + signal, + admissionTier, + timeoutMs: timeout, + // Only gitlink paths are retained, so index size is not a memory risk. + maxBuffer: Number.MAX_SAFE_INTEGER, + maxStderrBuffer: 64 * 1024, + onStdout: (chunk) => { + const entries = (carry + chunk).split('\0') + carry = entries.pop() ?? '' + entries.forEach(keepGitlink) + } + }) + keepGitlink(carry) + return gitlinks +} + /** Exclude only the untracked shared links tolerated by the caller. */ function getBlockingUntrackedStatusEntries( status: string, diff --git a/src/main/git/worktrees/removal/worktree-removal-real-git.test.ts b/src/main/git/worktrees/removal/worktree-removal-real-git.test.ts index 763e20f94de..10bff011570 100644 --- a/src/main/git/worktrees/removal/worktree-removal-real-git.test.ts +++ b/src/main/git/worktrees/removal/worktree-removal-real-git.test.ts @@ -13,6 +13,7 @@ import { removeTree } from '../../../../shared/windows-transient-lock-removal' import { listWorktreesStrict } from '../catalog/worktree-listing' import { removeWorktree } from './worktree-removal' +import { assertWorktreeCleanForRemoval } from './worktree-removal-preflight' import { areWorktreePathsEqual } from '../catalog/worktree-path-comparison' import { isPrunableGitFileWorktree } from '../../../workspaces/removal/worktree-prunable-git-file' import { removeStaleLocalWorktreeRegistration } from '../../../workspaces/removal/local-worktree-removal-recovery' @@ -146,6 +147,31 @@ describe('worktree removal against the real Git binary', () => { expect(await isRegistered(subWorktreePath)).toBe(false) }) + // Builds a ~48k-entry real Git index (>10 MiB of ls-files output), which is slow on Windows runners. + it('finds a checked-out submodule in an index listing larger than the Git output buffer', async () => { + const blob = (await git(['hash-object', '-w', 'seed.txt'], worktreePath)).trim() + const head = (await git(['rev-parse', 'HEAD'], worktreePath)).trim() + const entries = Array.from( + { length: 48_000 }, + (_, i) => `100644 ${blob}\t${'nested-directory/'.repeat(12)}file-${i}.txt\n` + ) + // Sorts last, so the gitlink is only seen after more than 10 MiB of listing. + entries.push(`160000 ${head}\tzz-vendor\n`) + const indexInfo = execFileAsync('git', ['update-index', '--index-info'], { cwd: worktreePath }) + indexInfo.child.stdin?.end(entries.join('')) + await indexInfo + await mkdir(join(worktreePath, 'zz-vendor', '.git'), { recursive: true }) + + await expect( + assertWorktreeCleanForRemoval( + worktreePath, + false, + {}, + join(repoPath, '.git', 'worktrees', 'feature') + ) + ).rejects.toThrow('Worktree has uncommitted or untracked changes, and it contains submodules.') + }, 60_000) + it('prunes a proven malformed registration while retaining checkout files and its branch', async () => { const markerPath = join(worktreePath, '.git') const marker = await readFile(markerPath, 'utf8')