diff --git a/src/main/git/check-ignored-paths.test.ts b/src/main/git/check-ignored-paths.test.ts index 398d7c18650..44b690ebe57 100644 --- a/src/main/git/check-ignored-paths.test.ts +++ b/src/main/git/check-ignored-paths.test.ts @@ -44,6 +44,14 @@ describe('checkIgnoredPaths', () => { expect(gitExecFileAsyncMock.mock.calls[0]?.[1].stdin?.split('\0')).toHaveLength(10_001) }) + it('accepts a shorter caller-owned deadline', async () => { + gitExecFileAsyncMock.mockResolvedValue({ stdout: '.env\0', stderr: '' }) + + await expect(checkIgnoredPaths('/repo', ['.env'], {}, 250)).resolves.toEqual(['.env']) + + expect(gitExecFileAsyncMock.mock.calls[0]?.[1]).toMatchObject({ timeout: 250 }) + }) + it('treats exit code 1 as no ignored paths', async () => { gitExecFileAsyncMock.mockRejectedValue(Object.assign(new Error('no matches'), { code: 1 })) diff --git a/src/main/git/check-ignored-paths.ts b/src/main/git/check-ignored-paths.ts index 65e2a88f73d..3da5b1071ec 100644 --- a/src/main/git/check-ignored-paths.ts +++ b/src/main/git/check-ignored-paths.ts @@ -14,13 +14,14 @@ type GitExecError = Error & { stdout?: string; code?: number | string } async function runCheckIgnoredPaths( worktreePath: string, relativePaths: string[], - options: GitRuntimeOptions + options: GitRuntimeOptions, + timeoutMs: number ): Promise { try { const { stdout } = await gitExecFileAsync([...GIT_CHECK_IGNORE_STDIN_ARGS], { ...gitOptionsForWorktree(worktreePath, options), stdin: encodeGitCheckIgnorePaths(relativePaths), - timeout: GIT_CHECK_IGNORE_TIMEOUT_MS + timeout: timeoutMs }) return parseGitCheckIgnorePaths(stdout) } catch (error) { @@ -35,14 +36,15 @@ async function runCheckIgnoredPaths( export async function checkIgnoredPaths( worktreePath: string, relativePaths: string[], - options: GitRuntimeOptions = {} + options: GitRuntimeOptions = {}, + timeoutMs: number = GIT_CHECK_IGNORE_TIMEOUT_MS ): Promise { if (relativePaths.length === 0) { return [] } const ignored = new Set() for (const chunk of splitGitCheckIgnorePathsByStdinBytes(relativePaths)) { - for (const ignoredPath of await runCheckIgnoredPaths(worktreePath, chunk, options)) { + for (const ignoredPath of await runCheckIgnoredPaths(worktreePath, chunk, options, timeoutMs)) { ignored.add(ignoredPath) } } diff --git a/src/main/git/worktree-include-file.test.ts b/src/main/git/worktree-include-file.test.ts index 8da89965e7b..f4624d4ea04 100644 --- a/src/main/git/worktree-include-file.test.ts +++ b/src/main/git/worktree-include-file.test.ts @@ -2,8 +2,9 @@ import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' 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 { resolveWorktreeIncludePaths } from './worktree-include-file' import { matchesWorktreeIncludeGlob } from './worktree-include-glob' +import { parseWorktreeIncludePatterns } from './worktree-include-pattern' import { gitExecFileAsync } from './runner' vi.mock('./runner', () => ({ @@ -12,12 +13,19 @@ vi.mock('./runner', () => ({ const gitExecFileAsyncMock = vi.mocked(gitExecFileAsync) -/** Dispatch on subcommand: ls-files returns `entries`; check-ignore echoes back - * every stdin path present in `ignored`. */ -function mockGit(options: { entries?: string[]; ignored?: string[] }): void { +/** Dispatch on subcommand: check-ignore echoes every stdin path present in `ignored`. */ +function mockGit(options: { + entries?: string[] + collapsedEntries?: string[] + targetedEntries?: string[] + ignored?: string[] +}): void { gitExecFileAsyncMock.mockImplementation(async (args, execOptions) => { if (args.includes('ls-files')) { - return { stdout: (options.entries ?? []).map((entry) => `${entry}\0`).join(''), stderr: '' } + const entries = args.includes('--directory') + ? (options.collapsedEntries ?? options.entries ?? []) + : (options.targetedEntries ?? options.entries ?? []) + return { stdout: entries.map((entry) => `${entry}\0`).join(''), stderr: '' } } if (args.includes('check-ignore')) { const requested = (execOptions.stdin ?? '').split('\0').filter(Boolean) @@ -66,6 +74,12 @@ describe('parseWorktreeIncludePatterns', () => { ).toBe(true) expect(glob.glob && matchesWorktreeIncludeGlob(glob.glob, longNonMatch)).toBe(false) }) + + it('rejects pathological pattern counts before worktree creation can fan out', () => { + expect(() => parseWorktreeIncludePatterns(`${'.env\n'.repeat(4_097)}`)).toThrow( + 'contains too many patterns' + ) + }) }) describe('resolveWorktreeIncludePaths', () => { @@ -94,7 +108,7 @@ describe('resolveWorktreeIncludePaths', () => { writeFileSync(join(repo, '.env'), 'A=1') mkdirSync(join(repo, 'config')) writeFileSync(join(repo, 'config', 'secrets.json'), '{}') - mockGit({ ignored: ['.env', 'config/secrets.json'] }) + mockGit({ entries: ['.env'], ignored: ['.env', 'config/secrets.json'] }) await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual([ '.env', @@ -118,6 +132,20 @@ describe('resolveWorktreeIncludePaths', () => { expect(lsFilesCalls[1][0]).toContain(':(glob)**/.env') }) + it('routes every Git probe through the selected WSL runtime', async () => { + writeInclude('.env\n') + mockGit({ entries: ['.env'], ignored: ['.env'] }) + + await expect(resolveWorktreeIncludePaths(repo, { wslDistro: 'Ubuntu' })).resolves.toEqual([ + '.env' + ]) + + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(3) + expect( + gitExecFileAsyncMock.mock.calls.every(([, options]) => options.wslDistro === 'Ubuntu') + ).toBe(true) + }) + it('avoids enumeration for root-anchored literal patterns', async () => { writeInclude('/config/secrets.json\n') mkdirSync(join(repo, 'config')) @@ -132,7 +160,7 @@ describe('resolveWorktreeIncludePaths', () => { writeInclude('.env\ntracked.json\n') writeFileSync(join(repo, '.env'), 'A=1') writeFileSync(join(repo, 'tracked.json'), '{}') - mockGit({ ignored: ['.env'] }) + mockGit({ entries: ['.env'], ignored: ['.env'] }) await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual(['.env']) }) @@ -160,10 +188,63 @@ describe('resolveWorktreeIncludePaths', () => { await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual([]) rmSync(join(repo, 'logs')) mkdirSync(join(repo, 'logs')) - mockGit({ ignored: ['logs'] }) + mockGit({ collapsedEntries: ['logs/'], ignored: ['logs'] }) await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual(['logs']) }) + it('finds a matching directory beneath a collapsed ignored parent', async () => { + writeInclude('build/\n') + mockGit({ + collapsedEntries: ['parent/'], + targetedEntries: ['parent/build/marker'], + ignored: ['parent/build/marker'] + }) + + await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual(['parent/build/marker']) + const targetedArgs = gitExecFileAsyncMock.mock.calls.find( + ([args]) => args.includes('ls-files') && !args.includes('--directory') + )?.[0] + expect(targetedArgs).toContain(':(glob)**/build/**') + }) + + it('does not expand a matched directory during the targeted scan', async () => { + writeInclude('node_modules/\n') + mockGit({ collapsedEntries: ['node_modules/'], ignored: ['node_modules'] }) + + await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual(['node_modules']) + const targetedArgs = gitExecFileAsyncMock.mock.calls.find( + ([args]) => args.includes('ls-files') && !args.includes('--directory') + )?.[0] + expect(targetedArgs).toContain(':(glob)**/node_modules/**') + expect(targetedArgs).toContain(':(exclude,literal)node_modules') + }) + + it('does not let an unignored literal parent hide an ignored nested match', async () => { + writeInclude('/config\n.env\n') + mkdirSync(join(repo, 'config')) + mockGit({ entries: ['config/.env'], ignored: ['config/.env'] }) + + await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual(['config/.env']) + }) + + it('bounds the number of paths that can reach copy materialization', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + writeInclude('*\n') + mockGit({ + collapsedEntries: Array.from({ length: 10_001 }, (_, index) => `ignored-${index}`) + }) + + await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual([]) + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('Failed to resolve'), + expect.objectContaining({ message: expect.stringContaining('matched too many paths') }) + ) + expect(gitExecFileAsyncMock.mock.calls.some(([args]) => args.includes('check-ignore'))).toBe( + false + ) + warn.mockRestore() + }) + it('applies negations with last-match-wins semantics', async () => { writeInclude('.env.*\n!.env.production\n') mockGit({ @@ -177,7 +258,7 @@ describe('resolveWorktreeIncludePaths', () => { it('rejects traversal, absolute, and .git patterns', async () => { writeInclude('../outside\n/etc/passwd\n.git/config\n.env\n') writeFileSync(join(repo, '.env'), 'A=1') - mockGit({ ignored: ['.env'] }) + mockGit({ entries: ['.env'], ignored: ['.env'] }) await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual(['.env']) }) diff --git a/src/main/git/worktree-include-file.ts b/src/main/git/worktree-include-file.ts index 97c33399602..685649ca149 100644 --- a/src/main/git/worktree-include-file.ts +++ b/src/main/git/worktree-include-file.ts @@ -5,11 +5,11 @@ 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' + isIncludedByWorktreePatterns, + parseWorktreeIncludePatterns, + WORKTREE_INCLUDE_MATCH_STEP_BUDGET, + type WorktreeIncludePattern +} from './worktree-include-pattern' /** Project-level list of gitignored paths to copy into each new worktree. * Cross-tool convention (gitignore syntax); see issue #7549. */ @@ -19,96 +19,12 @@ 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 +const WORKTREE_INCLUDE_MAX_CANDIDATES = 10_000 +const WORKTREE_INCLUDE_MAX_CANDIDATE_BYTES = 1024 * 1024 // 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 -type WorktreeIncludePattern = { - negated: boolean - /** Pattern with `!`, leading `/`, and trailing `/` stripped. */ - body: string - dirOnly: boolean - anchored: boolean - hasGlob: boolean - glob: CompiledWorktreeIncludeGlob | null -} - -export function parseWorktreeIncludePatterns(content: string): WorktreeIncludePattern[] { - const patterns: WorktreeIncludePattern[] = [] - for (const rawLine of content.split(/\r?\n/)) { - const line = rawLine.trim() - if (!line || line.startsWith('#')) { - continue - } - const negated = line.startsWith('!') - const withoutNegation = negated ? line.slice(1) : line - const dirOnly = withoutNegation.endsWith('/') - // Why: gitignore semantics — a slash anywhere except the end anchors the - // pattern to the repo root; a bare name matches at any depth. - const trimmed = withoutNegation.replace(/^\//, '').replace(/\/+$/, '') - if (!trimmed) { - continue - } - const anchored = withoutNegation.startsWith('/') || trimmed.includes('/') - const hasGlob = /[*?]/.test(trimmed) - patterns.push({ - negated, - body: trimmed, - dirOnly, - anchored, - hasGlob, - glob: hasGlob ? compileWorktreeIncludeGlob(trimmed) : null - }) - } - return patterns -} - -function patternMatches( - pattern: WorktreeIncludePattern, - relativePath: string, - isDirectory: boolean, - globBudget: { remaining: number } -): boolean { - if (pattern.dirOnly && !isDirectory) { - return false - } - // Why: like gitignore, a matched directory brings everything beneath it, so - // a path inside a matched prefix counts as matched. - const subject = pattern.anchored ? relativePath : (relativePath.split('/').at(-1) ?? relativePath) - if (!pattern.hasGlob) { - return ( - subject === pattern.body || (pattern.anchored && relativePath.startsWith(`${pattern.body}/`)) - ) - } - 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, - 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, globBudget)) { - included = !pattern.negated - } - } - return included -} - function isSafeIncludeCandidate(relativePath: string): boolean { if (!relativePath || isAbsolute(relativePath)) { return false @@ -120,6 +36,22 @@ function isSafeIncludeCandidate(relativePath: string): boolean { type GitignoredEntry = { relativePath: string isDirectory: boolean + fromIgnoredScan: boolean +} + +function hasCandidateDirectoryAncestor( + candidates: ReadonlyMap, + relativePath: string +): boolean { + let separatorIndex = relativePath.indexOf('/') + while (separatorIndex !== -1) { + const ancestor = candidates.get(relativePath.slice(0, separatorIndex)) + if (ancestor?.isDirectory && ancestor.fromIgnoredScan) { + return true + } + separatorIndex = relativePath.indexOf('/', separatorIndex + 1) + } + return false } type ListGitignoredEntriesOptions = { @@ -156,7 +88,8 @@ async function listGitignoredEntries( const isDirectory = rawEntry.endsWith('/') entries.push({ relativePath: isDirectory ? rawEntry.replace(/\/+$/, '') : rawEntry, - isDirectory + isDirectory, + fromIgnoredScan: true }) } return entries @@ -169,29 +102,41 @@ function patternToGitPathspec(pattern: WorktreeIncludePattern): string { return `:(glob)${pattern.anchored ? '' : '**/'}${body}` } -function chunkPathspecs(pathspecs: readonly string[]): string[][] { +function patternToGitDescendantPathspec(pattern: WorktreeIncludePattern): string { + return `${patternToGitPathspec(pattern)}/**` +} + +function chunkPathspecs( + pathspecs: readonly string[], + sharedPathspecs: readonly string[] = [] +): string[][] { const chunks: string[][] = [] let chunk: string[] = [] - let chunkLength = 0 + 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)) { // A single impossible-to-match pattern must not exceed Windows' process // command-line limit; the collapsed scan still provides safe fallback. - if (pathspec.length > WORKTREE_INCLUDE_PATHSPEC_CHUNK_LENGTH) { + if (pathspec.length + sharedLength > WORKTREE_INCLUDE_PATHSPEC_CHUNK_LENGTH) { continue } if ( chunk.length > 0 && chunkLength + pathspec.length > WORKTREE_INCLUDE_PATHSPEC_CHUNK_LENGTH ) { - chunks.push(chunk) + chunks.push([...chunk, ...shared]) chunk = [] - chunkLength = 0 + chunkLength = sharedLength } chunk.push(pathspec) chunkLength += pathspec.length } if (chunk.length > 0) { - chunks.push(chunk) + chunks.push([...chunk, ...shared]) } return chunks } @@ -234,25 +179,47 @@ export async function resolveWorktreeIncludePaths( } const candidates = new Map() - const globBudget = { remaining: WORKTREE_INCLUDE_GLOB_STEP_BUDGET } + const matchBudget = { remaining: WORKTREE_INCLUDE_MATCH_STEP_BUDGET } + const deadline = Date.now() + WORKTREE_INCLUDE_GIT_TIMEOUT_MS const addCandidate = (entry: GitignoredEntry): void => { if ( isSafeIncludeCandidate(entry.relativePath) && - isIncludedByPatterns(patterns, entry.relativePath, entry.isDirectory, globBudget) + isIncludedByWorktreePatterns(patterns, entry.relativePath, entry.isDirectory, matchBudget) ) { + if (hasCandidateDirectoryAncestor(candidates, entry.relativePath)) { + return + } + if ( + !candidates.has(entry.relativePath) && + candidates.size >= WORKTREE_INCLUDE_MAX_CANDIDATES + ) { + throw new Error(`${WORKTREE_INCLUDE_FILE} matched too many paths`) + } candidates.set(entry.relativePath, entry) } } - // Literal patterns resolve by direct stat so they work even for paths - // nested inside a wholly-gitignored directory that ls-files collapses. - for (const pattern of patterns) { - if (pattern.negated || pattern.hasGlob || !isSafeIncludeCandidate(pattern.body)) { - continue - } + // Why: anchored literals cannot be discovered more cheaply than a direct probe; + // broad literals use the bounded Git scans below instead of serial filesystem I/O. + const anchoredLiteralPaths = new Set( + patterns + .filter( + (pattern) => + !pattern.negated && + !pattern.hasGlob && + pattern.anchored && + isSafeIncludeCandidate(pattern.body) + ) + .map((pattern) => pattern.body) + ) + for (const relativePath of anchoredLiteralPaths) { try { - const stats = await lstat(join(repoPath, pattern.body)) - addCandidate({ relativePath: pattern.body, isDirectory: stats.isDirectory() }) + const stats = await lstat(join(repoPath, relativePath)) + addCandidate({ + relativePath, + isDirectory: stats.isDirectory(), + fromIgnoredScan: false + }) } catch { // Absent in the primary checkout — nothing to copy. } @@ -262,7 +229,6 @@ export async function resolveWorktreeIncludePaths( (pattern) => !pattern.negated && (pattern.hasGlob || !pattern.anchored) ) if (broadPatterns.length > 0) { - const deadline = Date.now() + WORKTREE_INCLUDE_GIT_TIMEOUT_MS // Why: collapsing ignored directories prevents huge trees such as // node_modules from producing one entry per file. for (const entry of await listGitignoredEntries(repoPath, options, { @@ -275,10 +241,18 @@ export async function resolveWorktreeIncludePaths( // Why: Git 2.25's directory collapsing can stop at an untracked parent // before discovering nested ignored files; targeted scans preserve the // cross-version behavior without expanding unrelated ignored trees. - const filePathspecs = broadPatterns - .filter((pattern) => !pattern.dirOnly) - .map(patternToGitPathspec) - for (const pathspecs of chunkPathspecs(filePathspecs)) { + const filePathspecs = broadPatterns.flatMap((pattern) => [ + ...(pattern.dirOnly ? [] : [patternToGitPathspec(pattern)]), + // Why: files beneath a matching directory expose directory matches that + // Git 2.25 otherwise hides behind a collapsed ignored parent. + patternToGitDescendantPathspec(pattern) + ]) + // Why: collapsed directories are already copied as units; excluding them + // keeps a targeted nested scan from expanding node_modules-scale trees. + const coveredDirectoryPathspecs = Array.from(candidates.values()) + .filter((entry) => entry.isDirectory && entry.fromIgnoredScan) + .map((entry) => `:(exclude,literal)${entry.relativePath}`) + for (const pathspecs of chunkPathspecs(filePathspecs, coveredDirectoryPathspecs)) { const timeout = deadline - Date.now() if (timeout <= 0) { throw new Error(`${WORKTREE_INCLUDE_FILE} Git enumeration timed out`) @@ -296,14 +270,26 @@ export async function resolveWorktreeIncludePaths( return [] } + 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`) + } + const checkIgnoreTimeout = deadline - Date.now() + if (checkIgnoreTimeout <= 0) { + throw new Error(`${WORKTREE_INCLUDE_FILE} resolution timed out`) + } + // 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( - await checkIgnoredPaths(repoPath, Array.from(candidates.keys()), options) + await checkIgnoredPaths(repoPath, candidatePaths, options, checkIgnoreTimeout) ) - return Array.from(candidates.keys()) - .filter((relativePath) => ignored.has(relativePath)) - .sort() + return candidatePaths.filter((relativePath) => ignored.has(relativePath)).sort() } catch (error) { console.warn(`[worktree-include] Failed to resolve ${WORKTREE_INCLUDE_FILE} patterns:`, error) return [] diff --git a/src/main/git/worktree-include-flow.test.ts b/src/main/git/worktree-include-flow.test.ts new file mode 100644 index 00000000000..722a6addbce --- /dev/null +++ b/src/main/git/worktree-include-flow.test.ts @@ -0,0 +1,65 @@ +import { execFile } from 'node:child_process' +import { lstat, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { promisify } from 'node:util' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { createWorktreeCopiedPaths } from '../ipc/worktree-symlinks' +import { resolveWorktreeIncludePaths } from './worktree-include-file' + +const execFileAsync = promisify(execFile) + +describe('worktree include creation flow', () => { + let root = '' + let primary = '' + let worktree = '' + + async function git(args: string[], cwd = primary): Promise { + await execFileAsync('git', args, { cwd }) + } + + beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-worktree-include-flow-')) + primary = join(root, 'primary') + worktree = join(root, 'feature') + await mkdir(primary) + await git(['init', '-q']) + await git(['config', 'user.email', 'worktree-include@example.invalid']) + await git(['config', 'user.name', 'Worktree Include Test']) + }) + + afterEach(async () => { + await rm(root, { recursive: true, force: true }) + }) + + it('copies only matched ignored paths and leaves ordinary removal clean', async () => { + await writeFile(join(primary, '.gitignore'), '.env\ncache/\nparent/\n') + await writeFile(join(primary, '.worktreeinclude'), '.env\ncache/\nbuild/\ntracked.txt\n') + await writeFile(join(primary, 'tracked.txt'), 'tracked\n') + await writeFile(join(primary, '.env'), 'ROOT=1\n') + await mkdir(join(primary, 'apps', 'web'), { recursive: true }) + await writeFile(join(primary, 'apps', 'web', '.env'), 'NESTED=1\n') + await mkdir(join(primary, 'cache')) + await writeFile(join(primary, 'cache', 'artifact'), 'cached\n') + await mkdir(join(primary, 'parent', 'build'), { recursive: true }) + await writeFile(join(primary, 'parent', 'build', 'marker'), 'built\n') + await git(['add', '.gitignore', '.worktreeinclude', 'tracked.txt']) + await git(['commit', '-qm', 'initial']) + await git(['worktree', 'add', '-q', '-b', 'feature', worktree]) + + const includePaths = await resolveWorktreeIncludePaths(primary) + expect(includePaths).toEqual(['.env', 'apps/web/.env', 'cache', 'parent/build/marker']) + + await createWorktreeCopiedPaths(primary, worktree, includePaths, { platform: 'linux' }) + + expect(await readFile(join(worktree, '.env'), 'utf8')).toBe('ROOT=1\n') + expect(await readFile(join(worktree, 'apps', 'web', '.env'), 'utf8')).toBe('NESTED=1\n') + expect(await readFile(join(worktree, 'cache', 'artifact'), 'utf8')).toBe('cached\n') + expect(await readFile(join(worktree, 'parent', 'build', 'marker'), 'utf8')).toBe('built\n') + expect((await lstat(join(worktree, 'tracked.txt'))).isFile()).toBe(true) + + await writeFile(join(worktree, '.env'), 'ROOT=2\n') + expect(await readFile(join(primary, '.env'), 'utf8')).toBe('ROOT=1\n') + await git(['worktree', 'remove', worktree]) + }) +}) diff --git a/src/main/git/worktree-include-glob.ts b/src/main/git/worktree-include-glob.ts index 9c70870a094..b9f362f5f56 100644 --- a/src/main/git/worktree-include-glob.ts +++ b/src/main/git/worktree-include-glob.ts @@ -50,7 +50,7 @@ export function getWorktreeIncludeGlobStepCount( pattern: CompiledWorktreeIncludeGlob, value: string ): number { - return pattern.regExp === null ? pattern.tokens.length * (value.length + 1) : 1 + return pattern.regExp === null ? pattern.tokens.length * (value.length + 1) : value.length + 1 } export function matchesWorktreeIncludeGlob( diff --git a/src/main/git/worktree-include-pattern.ts b/src/main/git/worktree-include-pattern.ts new file mode 100644 index 00000000000..acb62a975f8 --- /dev/null +++ b/src/main/git/worktree-include-pattern.ts @@ -0,0 +1,135 @@ +import { + compileWorktreeIncludeGlob, + getWorktreeIncludeGlobStepCount, + matchesWorktreeIncludeGlob, + type CompiledWorktreeIncludeGlob +} from './worktree-include-glob' + +const WORKTREE_INCLUDE_FILE = '.worktreeinclude' +const WORKTREE_INCLUDE_MAX_PATTERNS = 4_096 +export const WORKTREE_INCLUDE_MATCH_STEP_BUDGET = 5_000_000 + +export type WorktreeIncludePattern = { + negated: boolean + /** Pattern with `!`, leading `/`, and trailing `/` stripped. */ + body: string + dirOnly: boolean + anchored: boolean + hasGlob: boolean + glob: CompiledWorktreeIncludeGlob | null +} + +export type WorktreeIncludeMatchBudget = { remaining: number } + +export function parseWorktreeIncludePatterns(content: string): WorktreeIncludePattern[] { + const patterns: WorktreeIncludePattern[] = [] + for (const rawLine of content.split(/\r?\n/)) { + const line = rawLine.trim() + if (!line || line.startsWith('#')) { + continue + } + const negated = line.startsWith('!') + const withoutNegation = negated ? line.slice(1) : line + const dirOnly = withoutNegation.endsWith('/') + // Why: a slash anywhere except the end anchors a gitignore pattern to the repo root. + const trimmed = withoutNegation.replace(/^\//, '').replace(/\/+$/, '') + if (!trimmed) { + continue + } + const anchored = withoutNegation.startsWith('/') || trimmed.includes('/') + const hasGlob = /[*?]/.test(trimmed) + patterns.push({ + negated, + body: trimmed, + dirOnly, + anchored, + hasGlob, + glob: hasGlob ? compileWorktreeIncludeGlob(trimmed) : null + }) + if (patterns.length > WORKTREE_INCLUDE_MAX_PATTERNS) { + throw new Error(`${WORKTREE_INCLUDE_FILE} contains too many patterns`) + } + } + return patterns +} + +function spendMatchBudget(budget: WorktreeIncludeMatchBudget, steps: number): void { + budget.remaining -= steps + if (budget.remaining < 0) { + throw new Error(`${WORKTREE_INCLUDE_FILE} matching exceeded its CPU budget`) + } +} + +function globMatches( + pattern: WorktreeIncludePattern, + subject: string, + budget: WorktreeIncludeMatchBudget +): boolean { + if (pattern.glob === null) { + return false + } + spendMatchBudget(budget, getWorktreeIncludeGlobStepCount(pattern.glob, subject)) + return matchesWorktreeIncludeGlob(pattern.glob, subject) +} + +function patternMatches( + pattern: WorktreeIncludePattern, + relativePath: string, + isDirectory: boolean, + budget: WorktreeIncludeMatchBudget +): boolean { + if (!pattern.hasGlob) { + if (pattern.anchored) { + spendMatchBudget(budget, relativePath.length + 1) + return ( + (relativePath === pattern.body && (!pattern.dirOnly || isDirectory)) || + relativePath.startsWith(`${pattern.body}/`) + ) + } + const segments = relativePath.split('/') + const lastMatchableIndex = + pattern.dirOnly && !isDirectory ? segments.length - 2 : segments.length - 1 + spendMatchBudget(budget, relativePath.length + 1) + return segments.slice(0, lastMatchableIndex + 1).includes(pattern.body) + } + + const segments = relativePath.split('/') + const lastMatchableIndex = + pattern.dirOnly && !isDirectory ? segments.length - 2 : segments.length - 1 + if (lastMatchableIndex < 0) { + return false + } + if (!pattern.anchored) { + for (let index = 0; index <= lastMatchableIndex; index++) { + if (globMatches(pattern, segments[index], budget)) { + return true + } + } + return false + } + + let prefix = '' + for (let index = 0; index <= lastMatchableIndex; index++) { + prefix = prefix ? `${prefix}/${segments[index]}` : segments[index] + if (globMatches(pattern, prefix, budget)) { + return true + } + } + return false +} + +export function isIncludedByWorktreePatterns( + patterns: readonly WorktreeIncludePattern[], + relativePath: string, + isDirectory: boolean, + budget: WorktreeIncludeMatchBudget +): boolean { + // Why: gitignore patterns are order-sensitive, so negations must use last-match-wins. + let included = false + for (const pattern of patterns) { + if (patternMatches(pattern, relativePath, isDirectory, budget)) { + included = !pattern.negated + } + } + return included +} diff --git a/src/main/ipc/worktree-apfs-clone.ts b/src/main/ipc/worktree-apfs-clone.ts index e2ade9575c5..41cdb32a9ca 100644 --- a/src/main/ipc/worktree-apfs-clone.ts +++ b/src/main/ipc/worktree-apfs-clone.ts @@ -1,7 +1,7 @@ import { execFile } from 'node:child_process' import { randomUUID } from 'node:crypto' import { mkdir, stat, rm, link, rmdir, chmod } from 'node:fs/promises' -import { dirname, resolve } from 'node:path' +import { dirname, resolve, sep } from 'node:path' import { promisify } from 'node:util' type ExecFileAsync = ( @@ -29,7 +29,6 @@ type DarwinFilesystemInfo = { export type ApfsCloneFilesystemCache = Map> export type ApfsCloneOptions = { - dereferenceSymlinks?: boolean filesystemCache?: ApfsCloneFilesystemCache } @@ -141,8 +140,7 @@ async function cloneFileWithApfs( async function cloneDirectoryWithApfs( source: string, target: string, - deps: ApfsCloneDeps, - dereferenceSymlinks: boolean + deps: ApfsCloneDeps ): Promise { const sourceMode = (await stat(source)).mode & 0o777 try { @@ -159,14 +157,9 @@ 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', - ...(dereferenceSymlinks ? ['-L'] : []), - source, - dirname(target) - ]) + // Why: copy into the reserved target so a resolved top-level symlink whose + // target has another basename still lands at the requested path. + await deps.execFileAsync('/bin/cp', ['-n', '-c', '-R', `${source}${sep}.`, target]) await chmod(target, sourceMode) } catch (error) { // Why: remove only the empty reservation. If cp wrote anything, or another @@ -190,6 +183,6 @@ export async function cloneWorktreePathWithApfs( // 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(source, target, deps, options.dereferenceSymlinks === true) + ? cloneDirectoryWithApfs(source, target, deps) : cloneFileWithApfs(source, target, deps)) } diff --git a/src/main/ipc/worktree-symlinks.test.ts b/src/main/ipc/worktree-symlinks.test.ts index 58f433bc7fd..d71a794d27c 100644 --- a/src/main/ipc/worktree-symlinks.test.ts +++ b/src/main/ipc/worktree-symlinks.test.ts @@ -6,13 +6,14 @@ import { lstatSync, readlinkSync, readFileSync, + realpathSync, rmSync, symlinkSync, existsSync, chmodSync } from 'node:fs' import { tmpdir } from 'node:os' -import { join } from 'node:path' +import { join, sep } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createWorktreeCopiedPaths, @@ -302,7 +303,7 @@ describe('createWorktreeSymlinks', () => { apfsCloneDeps: deps }) - expect(cpArgs).toEqual(['-n', '-c', '-R', source, worktree]) + expect(cpArgs).toEqual(['-n', '-c', '-R', `${source}${sep}.`, target]) expect(readFileSync(join(target, 'primary-marker'), 'utf8')).toBe('USER\n') expect(warn).toHaveBeenCalledWith( expect.stringContaining('[worktree-symlinks] APFS clone-copy unavailable'), @@ -449,7 +450,7 @@ describe('createWorktreeCopiedPaths', () => { expect(readFileSync(join(primary, '.env.shared'), 'utf8')).toBe('SECRET=1\n') }) - posixIt('dereferences nested symlinks so copied directories remain independent', async () => { + posixIt('preserves nested symlinks instead of expanding external trees', async () => { mkdirSync(join(primary, 'config')) writeFileSync(join(primary, 'shared.json'), '{"owner":"primary"}') symlinkSync(join(primary, 'shared.json'), join(primary, 'config', 'settings.json')) @@ -457,9 +458,8 @@ describe('createWorktreeCopiedPaths', () => { 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"}') + expect(lstatSync(copiedSettings).isSymbolicLink()).toBe(true) + expect(readlinkSync(copiedSettings)).toBe(join(primary, 'shared.json')) }) it('creates parent directories lazily for nested paths', async () => { @@ -545,6 +545,30 @@ describe('createWorktreeCopiedPaths', () => { expect(readFileSync(join(worktree, '.env'), 'utf8')).toBe('CLONED=1\n') }) + posixIt('copies a symlinked directory to its requested APFS destination', async () => { + const realSource = join(primary, 'shared-config') + mkdirSync(realSource) + writeFileSync(join(realSource, 'settings.json'), '{}') + symlinkSync(realSource, join(primary, 'config'), 'dir') + const target = join(worktree, 'config') + let cpArgs: readonly string[] | undefined + const deps = createApfsCloneDeps({ + onCp: (args) => { + cpArgs = args + writeFileSync(join(target, 'settings.json'), '{}') + } + }) + + await createWorktreeCopiedPaths(primary, worktree, ['config'], { + platform: 'darwin', + apfsCloneDeps: deps + }) + + expect(cpArgs).toEqual(['-n', '-c', '-R', `${realpathSync(realSource)}${sep}.`, target]) + expect(readFileSync(join(target, 'settings.json'), 'utf8')).toBe('{}') + expect(existsSync(join(worktree, 'shared-config'))).toBe(false) + }) + it('probes an APFS volume once when copying multiple paths', async () => { mkdirSync(join(primary, 'config')) mkdirSync(join(primary, 'secrets')) @@ -562,7 +586,7 @@ describe('createWorktreeCopiedPaths', () => { 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) + expect(copyCalls.every(([, args]) => !args.includes('-L'))).toBe(true) }) }) diff --git a/src/main/ipc/worktree-symlinks.ts b/src/main/ipc/worktree-symlinks.ts index e461df36b8f..37d43841b89 100644 --- a/src/main/ipc/worktree-symlinks.ts +++ b/src/main/ipc/worktree-symlinks.ts @@ -95,7 +95,7 @@ async function copyWorktreePath( recursive: true, force: false, errorOnExist: false, - dereference: true + dereference: false }) if (process.platform !== 'win32') { await chmod(target, sourceMode) @@ -113,7 +113,7 @@ async function copyWorktreePath( recursive: true, force: false, errorOnExist: false, - dereference: true + dereference: false }) } @@ -140,8 +140,7 @@ async function createWorktreeLinkedPath( cloneSourceIsDirectory, options.apfsCloneDeps ?? defaultApfsCloneDeps, { - filesystemCache: apfsFilesystemCache, - dereferenceSymlinks: mode === 'copy' + filesystemCache: apfsFilesystemCache } )) await cloneWorktreePath(copySource, target, sourceIsDirectory) diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index abac397a9a1..2b471b006c5 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -175,12 +175,16 @@ describeBinaryCompatibility('real Git binary compatibility', () => { }) it('lists collapsed directories and targeted nested files for worktree includes', async () => { - await writeFile(join(repoPath, '.gitignore'), '.env\ncache/\n') + await writeFile(join(repoPath, '.gitignore'), '.env\ncache/\nbuild/\nparent/\n') await writeFile(join(repoPath, '.env'), 'ROOT=1\n') await mkdir(join(repoPath, 'apps', 'web'), { recursive: true }) await writeFile(join(repoPath, 'apps', 'web', '.env'), 'NESTED=1\n') await mkdir(join(repoPath, 'cache')) await writeFile(join(repoPath, 'cache', 'artifact'), 'ignored\n') + await mkdir(join(repoPath, 'build')) + await writeFile(join(repoPath, 'build', 'root-artifact'), 'ignored\n') + await mkdir(join(repoPath, 'parent', 'build'), { recursive: true }) + await writeFile(join(repoPath, 'parent', 'build', 'nested-artifact'), 'ignored\n') const collapsed = await runGit([ '-c', @@ -209,5 +213,21 @@ describeBinaryCompatibility('real Git binary compatibility', () => { ':(glob)**/.env' ]) expect(targeted.stdout.split('\0').filter(Boolean)).toEqual(['.env', 'apps/web/.env']) + + const targetedDirectory = await runGit([ + '-c', + 'core.quotePath=false', + 'ls-files', + '--others', + '--ignored', + '--exclude-standard', + '-z', + '--', + ':(glob)**/build/**', + ':(exclude,literal)build' + ]) + expect(targetedDirectory.stdout.split('\0').filter(Boolean)).toEqual([ + 'parent/build/nested-artifact' + ]) }) })