From 7f5f9f4d70badff84cb50908516bf4d44233c06c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 21 Jul 2026 17:55:10 -0700 Subject: [PATCH] fix(worktrees): bound include copy costs --- src/main/git/worktree-include-file.test.ts | 17 +++- src/main/git/worktree-include-file.ts | 65 ++++++---------- src/main/git/worktree-include-glob.ts | 90 ++++++++++++++++++++++ src/main/ipc/worktree-apfs-clone.ts | 55 +++++++++++-- src/main/ipc/worktree-symlinks.test.ts | 33 ++++++++ src/main/ipc/worktree-symlinks.ts | 29 +++++-- 6 files changed, 234 insertions(+), 55 deletions(-) create mode 100644 src/main/git/worktree-include-glob.ts diff --git a/src/main/git/worktree-include-file.test.ts b/src/main/git/worktree-include-file.test.ts index 00a8ce15f3f..8da89965e7b 100644 --- a/src/main/git/worktree-include-file.test.ts +++ b/src/main/git/worktree-include-file.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { parseWorktreeIncludePatterns, resolveWorktreeIncludePaths } from './worktree-include-file' +import { matchesWorktreeIncludeGlob } from './worktree-include-glob' import { gitExecFileAsync } from './runner' vi.mock('./runner', () => ({ @@ -50,8 +51,20 @@ describe('parseWorktreeIncludePatterns', () => { it('marks glob patterns and compiles their matcher', () => { const [glob] = parseWorktreeIncludePatterns('.env.*\n') expect(glob.hasGlob).toBe(true) - expect(glob.regExp?.test('.env.local')).toBe(true) - expect(glob.regExp?.test('.envrc')).toBe(false) + expect(glob.glob && matchesWorktreeIncludeGlob(glob.glob, '.env.local')).toBe(true) + expect(glob.glob && matchesWorktreeIncludeGlob(glob.glob, '.envrc')).toBe(false) + }) + + it('matches repeated recursive globs without regex backtracking', () => { + const [glob] = parseWorktreeIncludePatterns(`${'**/'.repeat(30)}secrets.json\n`) + const longNonMatch = `${'nested/'.repeat(500)}other.json` + + expect(glob.glob?.regExp).toBeNull() + expect(glob.glob && matchesWorktreeIncludeGlob(glob.glob, 'secrets.json')).toBe(true) + expect( + glob.glob && matchesWorktreeIncludeGlob(glob.glob, `${'nested/'.repeat(500)}secrets.json`) + ).toBe(true) + expect(glob.glob && matchesWorktreeIncludeGlob(glob.glob, longNonMatch)).toBe(false) }) }) diff --git a/src/main/git/worktree-include-file.ts b/src/main/git/worktree-include-file.ts index 4e60d2b42b4..97c33399602 100644 --- a/src/main/git/worktree-include-file.ts +++ b/src/main/git/worktree-include-file.ts @@ -4,6 +4,12 @@ import { checkIgnoredPaths } from './check-ignored-paths' import type { GitRuntimeOptions } from './git-runtime-options' import { gitOptionsForWorktree } from './git-runtime-options' import { gitExecFileAsync } from './runner' +import { + compileWorktreeIncludeGlob, + getWorktreeIncludeGlobStepCount, + matchesWorktreeIncludeGlob, + type CompiledWorktreeIncludeGlob +} from './worktree-include-glob' /** Project-level list of gitignored paths to copy into each new worktree. * Cross-tool convention (gitignore syntax); see issue #7549. */ @@ -13,6 +19,8 @@ export const WORKTREE_INCLUDE_FILE = '.worktreeinclude' // filesystem or a pathological repo; bail and create the worktree without it. const WORKTREE_INCLUDE_GIT_TIMEOUT_MS = 15_000 const WORKTREE_INCLUDE_MAX_FILE_BYTES = 256 * 1024 +// Why: glob filtering runs on the main process outside the Git subprocess deadline. +const WORKTREE_INCLUDE_GLOB_STEP_BUDGET = 5_000_000 // Why: Windows limits native command lines to 32K UTF-16 code units; leave // room for Git's executable, fixed arguments, and the repository path. const WORKTREE_INCLUDE_PATHSPEC_CHUNK_LENGTH = 12 * 1024 @@ -24,7 +32,7 @@ type WorktreeIncludePattern = { dirOnly: boolean anchored: boolean hasGlob: boolean - regExp: RegExp | null + glob: CompiledWorktreeIncludeGlob | null } export function parseWorktreeIncludePatterns(content: string): WorktreeIncludePattern[] { @@ -51,49 +59,17 @@ export function parseWorktreeIncludePatterns(content: string): WorktreeIncludePa dirOnly, anchored, hasGlob, - regExp: hasGlob ? gitignoreGlobToRegExp(trimmed) : null + glob: hasGlob ? compileWorktreeIncludeGlob(trimmed) : null }) } return patterns } -// Why: `[...]` character classes are intentionally treated as literals — the -// glob subset here mirrors what git itself matches for the overwhelmingly -// common `.env.*` / `**/secrets` shapes without risking a bad user pattern -// becoming an invalid or pathological RegExp. -function gitignoreGlobToRegExp(pattern: string): RegExp | null { - let regex = '' - for (let index = 0; index < pattern.length; index++) { - const char = pattern[index] - if (char === '*') { - if (pattern[index + 1] === '*') { - regex += '.*' - index++ - // Collapse `**/` so `foo/**/bar` also matches `foo/bar`. - if (pattern[index + 1] === '/') { - regex += '/?' - index++ - } - } else { - regex += '[^/]*' - } - } else if (char === '?') { - regex += '[^/]' - } else { - regex += char.replace(/[.+^${}()|[\]\\]/, '\\$&') - } - } - try { - return new RegExp(`^${regex}$`) - } catch { - return null - } -} - function patternMatches( pattern: WorktreeIncludePattern, relativePath: string, - isDirectory: boolean + isDirectory: boolean, + globBudget: { remaining: number } ): boolean { if (pattern.dirOnly && !isDirectory) { return false @@ -106,19 +82,27 @@ function patternMatches( subject === pattern.body || (pattern.anchored && relativePath.startsWith(`${pattern.body}/`)) ) } - return pattern.regExp !== null && pattern.regExp.test(subject) + if (pattern.glob === null) { + return false + } + globBudget.remaining -= getWorktreeIncludeGlobStepCount(pattern.glob, subject) + if (globBudget.remaining < 0) { + throw new Error(`${WORKTREE_INCLUDE_FILE} glob matching exceeded its CPU budget`) + } + return matchesWorktreeIncludeGlob(pattern.glob, subject) } function isIncludedByPatterns( patterns: readonly WorktreeIncludePattern[], relativePath: string, - isDirectory: boolean + isDirectory: boolean, + globBudget: { remaining: number } ): boolean { // Why: gitignore semantics — the last matching pattern wins, so `!` lines // can carve exceptions out of an earlier broad include. let included = false for (const pattern of patterns) { - if (patternMatches(pattern, relativePath, isDirectory)) { + if (patternMatches(pattern, relativePath, isDirectory, globBudget)) { included = !pattern.negated } } @@ -250,10 +234,11 @@ export async function resolveWorktreeIncludePaths( } const candidates = new Map() + const globBudget = { remaining: WORKTREE_INCLUDE_GLOB_STEP_BUDGET } const addCandidate = (entry: GitignoredEntry): void => { if ( isSafeIncludeCandidate(entry.relativePath) && - isIncludedByPatterns(patterns, entry.relativePath, entry.isDirectory) + isIncludedByPatterns(patterns, entry.relativePath, entry.isDirectory, globBudget) ) { candidates.set(entry.relativePath, entry) } diff --git a/src/main/git/worktree-include-glob.ts b/src/main/git/worktree-include-glob.ts new file mode 100644 index 00000000000..9c70870a094 --- /dev/null +++ b/src/main/git/worktree-include-glob.ts @@ -0,0 +1,90 @@ +type WorktreeIncludeGlobToken = + | { kind: 'literal'; value: string } + | { kind: 'one' } + | { kind: 'segment-star' } + | { kind: 'recursive-star' } + | { kind: 'optional-slash' } + +export type CompiledWorktreeIncludeGlob = { + regExp: RegExp | null + tokens: readonly WorktreeIncludeGlobToken[] +} + +export function compileWorktreeIncludeGlob(pattern: string): CompiledWorktreeIncludeGlob { + const tokens: WorktreeIncludeGlobToken[] = [] + let regex = '' + let starCount = 0 + for (let index = 0; index < pattern.length; index++) { + const char = pattern[index] + if (char === '*') { + starCount++ + if (pattern[index + 1] === '*') { + tokens.push({ kind: 'recursive-star' }) + regex += '.*' + index++ + if (pattern[index + 1] === '/') { + tokens.push({ kind: 'optional-slash' }) + regex += '/?' + index++ + } + } else { + tokens.push({ kind: 'segment-star' }) + regex += '[^/]*' + } + } else if (char === '?') { + tokens.push({ kind: 'one' }) + regex += '[^/]' + } else { + tokens.push({ kind: 'literal', value: char }) + regex += char.replace(/[.+^${}()|[\]\\]/, '\\$&') + } + } + return { + // Why: multiple variable-width groups can make JS regex backtracking exponential. + regExp: starCount <= 1 ? new RegExp(`^${regex}$`) : null, + tokens + } +} + +export function getWorktreeIncludeGlobStepCount( + pattern: CompiledWorktreeIncludeGlob, + value: string +): number { + return pattern.regExp === null ? pattern.tokens.length * (value.length + 1) : 1 +} + +export function matchesWorktreeIncludeGlob( + pattern: CompiledWorktreeIncludeGlob, + value: string +): boolean { + if (pattern.regExp !== null) { + return pattern.regExp.test(value) + } + + let previous = new Uint8Array(value.length + 1) + previous[0] = 1 + for (const token of pattern.tokens) { + const current = new Uint8Array(value.length + 1) + if (token.kind === 'segment-star' || token.kind === 'recursive-star') { + current[0] = previous[0] + for (let index = 1; index <= value.length; index++) { + const canConsume = token.kind === 'recursive-star' || value[index - 1] !== '/' + current[index] = previous[index] || (canConsume && current[index - 1]) ? 1 : 0 + } + } else if (token.kind === 'optional-slash') { + current[0] = previous[0] + for (let index = 1; index <= value.length; index++) { + current[index] = + previous[index] || (value[index - 1] === '/' && previous[index - 1]) ? 1 : 0 + } + } else { + for (let index = 1; index <= value.length; index++) { + const matches = + token.kind === 'one' ? value[index - 1] !== '/' : value[index - 1] === token.value + current[index] = previous[index - 1] && matches ? 1 : 0 + } + } + previous = current + } + return previous[value.length] === 1 +} diff --git a/src/main/ipc/worktree-apfs-clone.ts b/src/main/ipc/worktree-apfs-clone.ts index d7c1a08680d..e2ade9575c5 100644 --- a/src/main/ipc/worktree-apfs-clone.ts +++ b/src/main/ipc/worktree-apfs-clone.ts @@ -26,6 +26,13 @@ type DarwinFilesystemInfo = { filesystemName: string } +export type ApfsCloneFilesystemCache = Map> + +export type ApfsCloneOptions = { + dereferenceSymlinks?: boolean + filesystemCache?: ApfsCloneFilesystemCache +} + export class ApfsCloneUnavailableError extends Error { constructor(message: string) { super(message) @@ -67,14 +74,35 @@ async function getDarwinFilesystemInfo( } } +async function getCachedDarwinFilesystemInfo( + path: string, + deps: ApfsCloneDeps, + cache: ApfsCloneFilesystemCache +): Promise { + const deviceId = (await stat(path)).dev + const cached = cache.get(deviceId) + if (cached) { + return cached + } + const pending = getDarwinFilesystemInfo(path, deps) + cache.set(deviceId, pending) + try { + return await pending + } catch (error) { + cache.delete(deviceId) + throw error + } +} + async function assertSameApfsVolume( source: string, target: string, - deps: ApfsCloneDeps + deps: ApfsCloneDeps, + cache: ApfsCloneFilesystemCache ): Promise { const [sourceInfo, targetInfo] = await Promise.all([ - getDarwinFilesystemInfo(source, deps), - getDarwinFilesystemInfo(dirname(target), deps) + getCachedDarwinFilesystemInfo(source, deps, cache), + getCachedDarwinFilesystemInfo(dirname(target), deps, cache) ]) if ( sourceInfo.device !== targetInfo.device || @@ -113,7 +141,8 @@ async function cloneFileWithApfs( async function cloneDirectoryWithApfs( source: string, target: string, - deps: ApfsCloneDeps + deps: ApfsCloneDeps, + dereferenceSymlinks: boolean ): Promise { const sourceMode = (await stat(source)).mode & 0o777 try { @@ -130,7 +159,14 @@ async function cloneDirectoryWithApfs( try { // Why: the top-level directory is reserved before cp runs, so use `-n` // to keep a raced nested file from being overwritten during the copy. - await deps.execFileAsync('/bin/cp', ['-n', '-c', '-R', source, dirname(target)]) + await deps.execFileAsync('/bin/cp', [ + '-n', + '-c', + '-R', + ...(dereferenceSymlinks ? ['-L'] : []), + source, + dirname(target) + ]) await chmod(target, sourceMode) } catch (error) { // Why: remove only the empty reservation. If cp wrote anything, or another @@ -144,13 +180,16 @@ export async function cloneWorktreePathWithApfs( source: string, target: string, sourceIsDirectory: boolean, - deps: ApfsCloneDeps = defaultApfsCloneDeps + deps: ApfsCloneDeps = defaultApfsCloneDeps, + options: ApfsCloneOptions = {} ): Promise { const targetParent = dirname(target) await mkdir(targetParent, { recursive: true }) - await assertSameApfsVolume(source, target, deps) + await assertSameApfsVolume(source, target, deps, options.filesystemCache ?? new Map()) // Why: Node's COPYFILE_FICLONE_FORCE returns ENOSYS on macOS in our runtime, // while Darwin's cp exposes APFS clonefile via -c. Preflight the volume so // cp's non-APFS full-copy fallback cannot surprise users. - await (sourceIsDirectory ? cloneDirectoryWithApfs : cloneFileWithApfs)(source, target, deps) + await (sourceIsDirectory + ? cloneDirectoryWithApfs(source, target, deps, options.dereferenceSymlinks === true) + : cloneFileWithApfs(source, target, deps)) } diff --git a/src/main/ipc/worktree-symlinks.test.ts b/src/main/ipc/worktree-symlinks.test.ts index 8d40fc6640d..58f433bc7fd 100644 --- a/src/main/ipc/worktree-symlinks.test.ts +++ b/src/main/ipc/worktree-symlinks.test.ts @@ -449,6 +449,19 @@ describe('createWorktreeCopiedPaths', () => { expect(readFileSync(join(primary, '.env.shared'), 'utf8')).toBe('SECRET=1\n') }) + posixIt('dereferences nested symlinks so copied directories remain independent', async () => { + mkdirSync(join(primary, 'config')) + writeFileSync(join(primary, 'shared.json'), '{"owner":"primary"}') + symlinkSync(join(primary, 'shared.json'), join(primary, 'config', 'settings.json')) + + await createWorktreeCopiedPaths(primary, worktree, ['config'], { platform: 'linux' }) + + const copiedSettings = join(worktree, 'config', 'settings.json') + expect(lstatSync(copiedSettings).isSymbolicLink()).toBe(false) + writeFileSync(copiedSettings, '{"owner":"worktree"}') + expect(readFileSync(join(primary, 'shared.json'), 'utf8')).toBe('{"owner":"primary"}') + }) + it('creates parent directories lazily for nested paths', async () => { mkdirSync(join(primary, 'apps', 'web'), { recursive: true }) writeFileSync(join(primary, 'apps', 'web', '.env'), 'A=1') @@ -531,6 +544,26 @@ describe('createWorktreeCopiedPaths', () => { ) expect(readFileSync(join(worktree, '.env'), 'utf8')).toBe('CLONED=1\n') }) + + it('probes an APFS volume once when copying multiple paths', async () => { + mkdirSync(join(primary, 'config')) + mkdirSync(join(primary, 'secrets')) + const deps = createApfsCloneDeps({}) + + await createWorktreeCopiedPaths(primary, worktree, ['config', 'secrets'], { + platform: 'darwin', + apfsCloneDeps: deps + }) + + const execFileAsyncMock = vi.mocked(deps.execFileAsync) + const filesystemProbeCalls = execFileAsyncMock.mock.calls.filter( + ([file]) => file === '/bin/df' || file === '/usr/sbin/diskutil' + ) + expect(filesystemProbeCalls).toHaveLength(2) + const copyCalls = execFileAsyncMock.mock.calls.filter(([file]) => file === '/bin/cp') + expect(copyCalls).toHaveLength(2) + expect(copyCalls.every(([, args]) => args.includes('-L'))).toBe(true) + }) }) describe('removeWorktreeSymlinks', () => { diff --git a/src/main/ipc/worktree-symlinks.ts b/src/main/ipc/worktree-symlinks.ts index d7fae48272c..e461df36b8f 100644 --- a/src/main/ipc/worktree-symlinks.ts +++ b/src/main/ipc/worktree-symlinks.ts @@ -5,6 +5,7 @@ import { cloneWorktreePathWithApfs, defaultApfsCloneDeps, WorktreeLinkedPathTargetExistsError, + type ApfsCloneFilesystemCache, type ApfsCloneDeps } from './worktree-apfs-clone' @@ -90,7 +91,12 @@ async function copyWorktreePath( throw error } try { - await cp(source, target, { recursive: true, force: false, errorOnExist: false }) + await cp(source, target, { + recursive: true, + force: false, + errorOnExist: false, + dereference: true + }) if (process.platform !== 'win32') { await chmod(target, sourceMode) } @@ -103,7 +109,12 @@ async function copyWorktreePath( } // Why: force=false + errorOnExist=false skips (not clobbers) anything a racing // process placed at the target after the earlier existence preflight. - await cp(source, target, { recursive: true, force: false, errorOnExist: false }) + await cp(source, target, { + recursive: true, + force: false, + errorOnExist: false, + dereference: true + }) } async function createWorktreeLinkedPath( @@ -112,7 +123,8 @@ async function createWorktreeLinkedPath( sourceIsDirectory: boolean, sourceIsSymbolicLink: boolean, mode: WorktreeMaterializeMode, - options: WorktreeLinkedPathOptions + options: WorktreeLinkedPathOptions, + apfsFilesystemCache: ApfsCloneFilesystemCache ): Promise { // Why: copy mode promises independent contents; copying the link itself // would let edits in the worktree mutate its shared target. @@ -126,7 +138,11 @@ async function createWorktreeLinkedPath( cloneSource, cloneTarget, cloneSourceIsDirectory, - options.apfsCloneDeps ?? defaultApfsCloneDeps + options.apfsCloneDeps ?? defaultApfsCloneDeps, + { + filesystemCache: apfsFilesystemCache, + dereferenceSymlinks: mode === 'copy' + } )) await cloneWorktreePath(copySource, target, sourceIsDirectory) return @@ -157,6 +173,8 @@ async function materializeWorktreePaths( options: WorktreeLinkedPathOptions = {} ): Promise { 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() for (const rawPath of paths) { const safePath = getSafeRelativePath(rawPath) @@ -194,7 +212,8 @@ async function materializeWorktreePaths( sourceIsDirectory, sourceIsSymbolicLink, mode, - effectiveOptions + effectiveOptions, + apfsFilesystemCache ) } catch (error) { console.error(