fix(worktrees): preserve bounded include resolution

This commit is contained in:
Brennan Benson
2026-07-22 13:39:01 -07:00
parent 5284a2e6c8
commit 504a1ebd21
6 changed files with 116 additions and 21 deletions
@@ -243,6 +243,27 @@ describe('resolveWorktreeIncludePaths', () => {
expect(targetedArgs).toContain(':(exclude,literal)node_modules')
})
it('preserves nested matches when covered-directory exclusions exceed the command budget', async () => {
writeInclude('covered-*/\n.env\n')
const coveredDirectories = Array.from(
{ length: 500 },
(_, index) => `covered-${index.toString().padStart(4, '0')}`
)
mockGit({
collapsedEntries: coveredDirectories.map((path) => `${path}/`),
targetedEntries: ['hidden/.env']
})
const resolved = await resolveWorktreeIncludePaths(repo)
expect(resolved).toContain('hidden/.env')
const targetedArgs = gitExecFileAsyncMock.mock.calls.find(
([args]) => args.includes('ls-files') && !args.includes('--directory')
)?.[0]
expect(targetedArgs).toContain(':(glob)**/.env')
expect(targetedArgs?.some((arg) => arg.startsWith(':(exclude,literal)'))).toBe(false)
})
it('does not let an unignored literal parent hide an ignored nested match', async () => {
writeInclude('/config\n.env\n')
mkdirSync(join(repo, 'config'))
@@ -269,6 +290,28 @@ describe('resolveWorktreeIncludePaths', () => {
warn.mockRestore()
})
it('stops enumeration as soon as candidate paths exceed the byte budget', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
writeInclude('*\n')
mockGit({
collapsedEntries: Array.from(
{ length: 100 },
(_, index) => `ignored-${index}-${'a'.repeat(11_000)}`
),
targetedEntries: []
})
await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual([])
expect(warn).toHaveBeenCalledWith(
expect.stringContaining('Failed to resolve'),
expect.objectContaining({ message: expect.stringContaining('byte budget') })
)
expect(
gitExecFileAsyncMock.mock.calls.filter(([args]) => args.includes('ls-files'))
).toHaveLength(1)
warn.mockRestore()
})
it('applies negations with last-match-wins semantics', async () => {
writeInclude('.env.*\n!.env.production\n')
mockGit({
+7 -8
View File
@@ -90,6 +90,7 @@ export async function resolveWorktreeIncludePaths(
}
const candidates = new Map<string, GitignoredEntry>()
let candidateBytes = 0
const matchBudget = { remaining: WORKTREE_INCLUDE_MATCH_STEP_BUDGET }
const deadline = Date.now() + WORKTREE_INCLUDE_GIT_TIMEOUT_MS
const addCandidate = (entry: GitignoredEntry): void => {
@@ -106,6 +107,12 @@ export async function resolveWorktreeIncludePaths(
) {
throw new Error(`${WORKTREE_INCLUDE_FILE} matched too many paths`)
}
if (!candidates.has(entry.relativePath)) {
candidateBytes += Buffer.byteLength(entry.relativePath) + 1
if (candidateBytes > WORKTREE_INCLUDE_MAX_CANDIDATE_BYTES) {
throw new Error(`${WORKTREE_INCLUDE_FILE} matched paths exceed its byte budget`)
}
}
candidates.set(entry.relativePath, entry)
}
}
@@ -235,14 +242,6 @@ export async function resolveWorktreeIncludePaths(
}
const candidatePaths = Array.from(candidates.keys())
if (
candidatePaths.reduce(
(bytes, relativePath) => bytes + Buffer.byteLength(relativePath) + 1,
0
) > WORKTREE_INCLUDE_MAX_CANDIDATE_BYTES
) {
throw new Error(`${WORKTREE_INCLUDE_FILE} matched paths exceed its byte budget`)
}
// Why: enforce the gitignored-only contract for literal-derived candidates
// too — a listed-but-not-ignored path must not be copied (issue #7549).
const ignored = new Set(ignoredAnchoredDirectories)
@@ -0,0 +1,21 @@
import { describe, expect, it } from 'vitest'
import { chunkWorktreeIncludePathspecs } from './worktree-include-git-enumeration'
describe('chunkWorktreeIncludePathspecs', () => {
it('keeps shared exclusions when they fit the Windows-safe command budget', () => {
expect(
chunkWorktreeIncludePathspecs([':(glob)**/.env'], [':(exclude,literal)node_modules'])
).toEqual([[':(glob)**/.env', ':(exclude,literal)node_modules']])
})
it('keeps targeted scans when shared exclusions exceed the command budget', () => {
const exclusions = Array.from(
{ length: 1_000 },
(_, index) => `:(exclude,literal)generated-${index}`
)
expect(chunkWorktreeIncludePathspecs([':(glob)**/.env'], exclusions)).toEqual([
[':(glob)**/.env']
])
})
})
@@ -73,28 +73,38 @@ export function chunkWorktreeIncludePathspecs(
let chunk: string[] = []
const shared = Array.from(new Set(sharedPathspecs))
const sharedLength = shared.reduce((length, pathspec) => length + pathspec.length, 0)
if (sharedLength >= WORKTREE_INCLUDE_PATHSPEC_CHUNK_LENGTH) {
return chunks
}
let chunkLength = sharedLength
for (const pathspec of new Set(pathspecs)) {
const uniquePathspecs = Array.from(new Set(pathspecs))
// Why: dropping exclusions can cost a bounded rescan, but dropping the scan silently misses nested config files.
const effectiveShared =
sharedLength < WORKTREE_INCLUDE_PATHSPEC_CHUNK_LENGTH &&
uniquePathspecs.every(
(pathspec) => pathspec.length + sharedLength <= WORKTREE_INCLUDE_PATHSPEC_CHUNK_LENGTH
)
? shared
: []
const effectiveSharedLength = effectiveShared.reduce(
(length, pathspec) => length + pathspec.length,
0
)
let chunkLength = effectiveSharedLength
for (const pathspec of uniquePathspecs) {
// Why: the collapsed scan remains a safe fallback when one pattern cannot fit the Windows command line.
if (pathspec.length + sharedLength > WORKTREE_INCLUDE_PATHSPEC_CHUNK_LENGTH) {
if (pathspec.length + effectiveSharedLength > WORKTREE_INCLUDE_PATHSPEC_CHUNK_LENGTH) {
continue
}
if (
chunk.length > 0 &&
chunkLength + pathspec.length > WORKTREE_INCLUDE_PATHSPEC_CHUNK_LENGTH
) {
chunks.push([...chunk, ...shared])
chunks.push([...chunk, ...effectiveShared])
chunk = []
chunkLength = sharedLength
chunkLength = effectiveSharedLength
}
chunk.push(pathspec)
chunkLength += pathspec.length
}
if (chunk.length > 0) {
chunks.push([...chunk, ...shared])
chunks.push([...chunk, ...effectiveShared])
}
return chunks
}
+18
View File
@@ -623,6 +623,24 @@ describe('createWorktreeCopiedPaths', () => {
expect(warn).toHaveBeenCalledTimes(diskutilError ? 1 : 0)
}
)
it('logs repeated APFS clone failures once while copying every path', async () => {
writeFileSync(join(primary, '.env'), 'SECRET=1\n')
writeFileSync(join(primary, '.env.local'), 'LOCAL=1\n')
const cloneWorktreePath = vi.fn(async () => {
throw Object.assign(new Error('clonefile unsupported'), { code: 'ENOTSUP' })
})
await createWorktreeCopiedPaths(primary, worktree, ['.env', '.env.local'], {
platform: 'darwin',
cloneWorktreePath
})
expect(cloneWorktreePath).toHaveBeenCalledTimes(2)
expect(warn).toHaveBeenCalledTimes(1)
expect(readFileSync(join(worktree, '.env'), 'utf8')).toBe('SECRET=1\n')
expect(readFileSync(join(worktree, '.env.local'), 'utf8')).toBe('LOCAL=1\n')
})
})
describe('removeWorktreeSymlinks', () => {
+8 -4
View File
@@ -155,7 +155,7 @@ async function createWorktreeLinkedPath(
mode: WorktreeMaterializeMode,
options: WorktreeLinkedPathOptions,
apfsFilesystemCache: ApfsCloneFilesystemCache,
warnedApfsErrors: Set<unknown>,
warnedApfsErrors: Set<string>,
mergeExistingDirectory: boolean
): Promise<void> {
// Why: copy mode promises independent contents; copying the link itself
@@ -185,8 +185,12 @@ async function createWorktreeLinkedPath(
// Why: APFS clone-copy can fail across volumes or on non-APFS disks.
// Fall back per mode without touching any target path that may have
// appeared after our preflight.
if (!(error instanceof ApfsCloneUnavailableError) && !warnedApfsErrors.has(error)) {
warnedApfsErrors.add(error)
const errorKey =
error instanceof Error
? `${error.name}:${(error as NodeJS.ErrnoException).code ?? ''}:${error.message}`
: String(error)
if (!(error instanceof ApfsCloneUnavailableError) && !warnedApfsErrors.has(errorKey)) {
warnedApfsErrors.add(errorKey)
console.warn(`[worktree-symlinks] APFS clone-copy unavailable for "${target}":`, error)
}
}
@@ -208,7 +212,7 @@ async function materializeWorktreePaths(
const effectiveOptions = { platform: process.platform, ...options }
// Why: probing APFS via df+diskutil once per copied path multiplies subprocesses in large includes.
const apfsFilesystemCache: ApfsCloneFilesystemCache = new Map()
const warnedApfsErrors = new Set<unknown>()
const warnedApfsErrors = new Set<string>()
const normalizedLinkedPaths = (options.existingLinkedPaths ?? []).flatMap((rawPath) => {
const linkedPath = getSafeRelativePath(rawPath)
return linkedPath.safe