mirror of
https://github.com/stablyai/orca.git
synced 2026-10-28 08:02:54 +00:00
fix(worktrees): tighten include matching and scan bounds
This commit is contained in:
@@ -89,6 +89,21 @@ describe('parseWorktreeIncludePatterns', () => {
|
||||
expect(glob.glob && matchesWorktreeIncludeGlob(glob.glob, longNonMatch)).toBe(false)
|
||||
})
|
||||
|
||||
it('keeps recursive directory globs on segment boundaries', () => {
|
||||
const leading = compileWorktreeIncludeGlob('**/.env')
|
||||
const nested = compileWorktreeIncludeGlob('foo/**/bar')
|
||||
const adjacent = compileWorktreeIncludeGlob('foo/**bar')
|
||||
|
||||
expect(matchesWorktreeIncludeGlob(leading, '.env')).toBe(true)
|
||||
expect(matchesWorktreeIncludeGlob(leading, 'nested/.env')).toBe(true)
|
||||
expect(matchesWorktreeIncludeGlob(leading, 'my.env')).toBe(false)
|
||||
expect(matchesWorktreeIncludeGlob(nested, 'foo/bar')).toBe(true)
|
||||
expect(matchesWorktreeIncludeGlob(nested, 'foo/nested/bar')).toBe(true)
|
||||
expect(matchesWorktreeIncludeGlob(nested, 'foo/xbar')).toBe(false)
|
||||
expect(matchesWorktreeIncludeGlob(adjacent, 'foo/xbar')).toBe(true)
|
||||
expect(matchesWorktreeIncludeGlob(adjacent, 'foo/x/bar')).toBe(false)
|
||||
})
|
||||
|
||||
it('matches literals and globs case-insensitively when the repository requires it', () => {
|
||||
const literal = parseWorktreeIncludePatterns('.env\n')
|
||||
const glob = parseWorktreeIncludePatterns('config/**/secret.json\n')
|
||||
@@ -304,6 +319,32 @@ describe('resolveWorktreeIncludePaths', () => {
|
||||
expect(targetedArgs).toContain(':(exclude,literal)node_modules')
|
||||
})
|
||||
|
||||
it('does not expand a collapsed directory excluded by trailing negation', async () => {
|
||||
writeInclude('*\n!node_modules/\n')
|
||||
mockGit({ collapsedEntries: ['node_modules/', 'cache/'], ignored: ['cache'] })
|
||||
|
||||
await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual(['cache'])
|
||||
const targetedArgs = gitExecFileAsyncMock.mock.calls.find(
|
||||
([args]) => args.includes('ls-files') && !args.includes('--directory')
|
||||
)?.[0]
|
||||
expect(targetedArgs).toContain(':(exclude,literal)node_modules')
|
||||
})
|
||||
|
||||
it('scans an excluded directory when a later include can restore a descendant', async () => {
|
||||
writeInclude('*\n!node_modules/\n.env\n')
|
||||
mockGit({
|
||||
collapsedEntries: ['node_modules/'],
|
||||
targetedEntries: ['node_modules/pkg/.env'],
|
||||
ignored: ['node_modules/pkg/.env']
|
||||
})
|
||||
|
||||
await expect(resolveWorktreeIncludePaths(repo)).resolves.toEqual(['node_modules/pkg/.env'])
|
||||
const targetedArgs = gitExecFileAsyncMock.mock.calls.find(
|
||||
([args]) => args.includes('ls-files') && !args.includes('--directory')
|
||||
)?.[0]
|
||||
expect(targetedArgs).not.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(
|
||||
|
||||
@@ -11,7 +11,7 @@ import {
|
||||
type GitignoredEntry
|
||||
} from './worktree-include-git-enumeration'
|
||||
import {
|
||||
isIncludedByWorktreePatterns,
|
||||
getWorktreeIncludePatternDecision,
|
||||
parseWorktreeIncludePatterns,
|
||||
WORKTREE_INCLUDE_MATCH_STEP_BUDGET,
|
||||
type WorktreeIncludePattern
|
||||
@@ -121,6 +121,7 @@ export async function resolveWorktreeIncludePaths(
|
||||
const ignoreCase = await getWorktreeIncludeIgnoreCase(repoPath, options, ignoreCaseTimeout)
|
||||
|
||||
const candidates = new Map<string, GitignoredEntry>()
|
||||
const excludedCoveredDirectories = new Set<string>()
|
||||
let candidateBytes = 0
|
||||
const matchBudget = { remaining: WORKTREE_INCLUDE_MATCH_STEP_BUDGET }
|
||||
const pruneCoveredCandidateDescendants = (): void => {
|
||||
@@ -138,33 +139,39 @@ export async function resolveWorktreeIncludePaths(
|
||||
}
|
||||
}
|
||||
const addCandidate = (entry: GitignoredEntry): void => {
|
||||
if (
|
||||
isSafeIncludeCandidate(entry.relativePath) &&
|
||||
isIncludedByWorktreePatterns(
|
||||
patterns,
|
||||
entry.relativePath,
|
||||
entry.isDirectory,
|
||||
matchBudget,
|
||||
ignoreCase
|
||||
)
|
||||
) {
|
||||
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`)
|
||||
}
|
||||
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)
|
||||
if (!isSafeIncludeCandidate(entry.relativePath)) {
|
||||
return
|
||||
}
|
||||
const decision = getWorktreeIncludePatternDecision(
|
||||
patterns,
|
||||
entry.relativePath,
|
||||
entry.isDirectory,
|
||||
matchBudget,
|
||||
ignoreCase
|
||||
)
|
||||
if (!decision.included) {
|
||||
if (entry.isDirectory && entry.coversDescendants && decision.excludesDescendants) {
|
||||
excludedCoveredDirectories.add(entry.relativePath)
|
||||
}
|
||||
return
|
||||
}
|
||||
excludedCoveredDirectories.delete(entry.relativePath)
|
||||
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`)
|
||||
}
|
||||
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)
|
||||
}
|
||||
|
||||
// Why: anchored literals cannot be discovered more cheaply than a direct probe;
|
||||
@@ -273,6 +280,12 @@ export async function resolveWorktreeIncludePaths(
|
||||
const coveredDirectoryPathspecs = Array.from(candidates.values())
|
||||
.filter((entry) => entry.isDirectory && entry.coversDescendants)
|
||||
.map((entry) => `:(exclude,literal)${entry.relativePath}`)
|
||||
coveredDirectoryPathspecs.push(
|
||||
...Array.from(
|
||||
excludedCoveredDirectories,
|
||||
(relativePath) => `:(exclude,literal)${relativePath}`
|
||||
)
|
||||
)
|
||||
for (const pathspecs of chunkWorktreeIncludePathspecs(
|
||||
filePathspecs,
|
||||
coveredDirectoryPathspecs
|
||||
|
||||
@@ -34,13 +34,14 @@ describe('worktree include creation flow', () => {
|
||||
|
||||
it('copies only matched ignored paths and leaves ordinary removal clean', async () => {
|
||||
await git(['config', 'core.ignoreCase', 'true'])
|
||||
await writeFile(join(primary, '.gitignore'), '.env\ncache/\nparent/\nmixed/local.txt\n')
|
||||
await writeFile(join(primary, '.gitignore'), '.env\nmy.env\ncache/\nparent/\nmixed/local.txt\n')
|
||||
await writeFile(
|
||||
join(primary, '.worktreeinclude'),
|
||||
'.env\ncache/\nbuild/\n/mixed/\ntracked.txt\n'
|
||||
'**/.env\ncache/\nbuild/\n/mixed/\ntracked.txt\n'
|
||||
)
|
||||
await writeFile(join(primary, 'tracked.txt'), 'tracked\n')
|
||||
await writeFile(join(primary, '.env'), 'ROOT=1\n')
|
||||
await writeFile(join(primary, 'my.env'), 'NOT_INCLUDED=1\n')
|
||||
await mkdir(join(primary, 'apps', 'web'), { recursive: true })
|
||||
await writeFile(join(primary, 'apps', 'web', '.env'), 'NESTED=1\n')
|
||||
await mkdir(join(primary, 'case'))
|
||||
|
||||
@@ -3,7 +3,7 @@ type WorktreeIncludeGlobToken =
|
||||
| { kind: 'one' }
|
||||
| { kind: 'segment-star' }
|
||||
| { kind: 'recursive-star' }
|
||||
| { kind: 'optional-slash' }
|
||||
| { kind: 'recursive-directory' }
|
||||
|
||||
export type CompiledWorktreeIncludeGlob = {
|
||||
regExp: RegExp | null
|
||||
@@ -20,18 +20,24 @@ export function compileWorktreeIncludeGlob(pattern: string): CompiledWorktreeInc
|
||||
const char = pattern[index]
|
||||
if (char === '*') {
|
||||
starCount++
|
||||
if (pattern[index + 1] === '*') {
|
||||
let starEnd = index
|
||||
while (pattern[starEnd + 1] === '*') {
|
||||
starEnd++
|
||||
}
|
||||
if (starEnd > index && pattern[starEnd + 1] === '/') {
|
||||
// Why: `**/` consumes complete directory prefixes; making only the slash
|
||||
// optional incorrectly lets `foo/**/bar` match `foo/xbar`.
|
||||
tokens.push({ kind: 'recursive-directory' })
|
||||
regex += '(?:.*/)?'
|
||||
index = starEnd + 1
|
||||
} else if (starEnd > index && starEnd === pattern.length - 1) {
|
||||
tokens.push({ kind: 'recursive-star' })
|
||||
regex += '.*'
|
||||
index++
|
||||
if (pattern[index + 1] === '/') {
|
||||
tokens.push({ kind: 'optional-slash' })
|
||||
regex += '/?'
|
||||
index++
|
||||
}
|
||||
index = starEnd
|
||||
} else {
|
||||
tokens.push({ kind: 'segment-star' })
|
||||
regex += '[^/]*'
|
||||
index = starEnd
|
||||
}
|
||||
} else if (char === '?') {
|
||||
tokens.push({ kind: 'one' })
|
||||
@@ -42,7 +48,10 @@ export function compileWorktreeIncludeGlob(pattern: string): CompiledWorktreeInc
|
||||
}
|
||||
}
|
||||
const variableWidthIndex = tokens.findIndex(
|
||||
(token) => token.kind === 'segment-star' || token.kind === 'recursive-star'
|
||||
(token) =>
|
||||
token.kind === 'segment-star' ||
|
||||
token.kind === 'recursive-star' ||
|
||||
token.kind === 'recursive-directory'
|
||||
)
|
||||
return {
|
||||
// Why: multiple variable-width groups can make JS regex backtracking exponential.
|
||||
@@ -92,11 +101,13 @@ export function matchesWorktreeIncludeGlob(
|
||||
const canConsume = token.kind === 'recursive-star' || comparableValue[index - 1] !== '/'
|
||||
current[index] = previous[index] || (canConsume && current[index - 1]) ? 1 : 0
|
||||
}
|
||||
} else if (token.kind === 'optional-slash') {
|
||||
} else if (token.kind === 'recursive-directory') {
|
||||
current[0] = previous[0]
|
||||
let canConsume = previous[0] === 1
|
||||
for (let index = 1; index <= value.length; index++) {
|
||||
canConsume ||= previous[index - 1] === 1
|
||||
current[index] =
|
||||
previous[index] || (comparableValue[index - 1] === '/' && previous[index - 1]) ? 1 : 0
|
||||
previous[index] || (canConsume && comparableValue[index - 1] === '/') ? 1 : 0
|
||||
}
|
||||
} else {
|
||||
for (let index = 1; index <= value.length; index++) {
|
||||
|
||||
@@ -22,6 +22,11 @@ export type WorktreeIncludePattern = {
|
||||
|
||||
export type WorktreeIncludeMatchBudget = { remaining: number }
|
||||
|
||||
export type WorktreeIncludePatternDecision = {
|
||||
included: boolean
|
||||
excludesDescendants: boolean
|
||||
}
|
||||
|
||||
export function parseWorktreeIncludePatterns(content: string): WorktreeIncludePattern[] {
|
||||
const patterns: WorktreeIncludePattern[] = []
|
||||
for (const rawLine of content.split(/\r?\n/)) {
|
||||
@@ -125,6 +130,40 @@ function patternMatches(
|
||||
return false
|
||||
}
|
||||
|
||||
export function getWorktreeIncludePatternDecision(
|
||||
patterns: readonly WorktreeIncludePattern[],
|
||||
relativePath: string,
|
||||
isDirectory: boolean,
|
||||
budget: WorktreeIncludeMatchBudget,
|
||||
ignoreCase: boolean = false
|
||||
): WorktreeIncludePatternDecision {
|
||||
// Why: gitignore patterns are order-sensitive, so negations must use last-match-wins.
|
||||
let included = false
|
||||
let lastMatchedIndex = -1
|
||||
let lastPositiveIndex = -1
|
||||
const caseFoldedRelativePath = ignoreCase ? relativePath.toLowerCase() : relativePath
|
||||
for (let index = 0; index < patterns.length; index++) {
|
||||
const pattern = patterns[index]
|
||||
if (!pattern.negated) {
|
||||
lastPositiveIndex = index
|
||||
}
|
||||
if (
|
||||
patternMatches(pattern, relativePath, caseFoldedRelativePath, isDirectory, budget, ignoreCase)
|
||||
) {
|
||||
included = !pattern.negated
|
||||
lastMatchedIndex = index
|
||||
}
|
||||
}
|
||||
return {
|
||||
included,
|
||||
// Why: a final directory exclusion covers descendants too, so Git need not expand the collapsed tree.
|
||||
excludesDescendants:
|
||||
lastMatchedIndex >= 0 &&
|
||||
patterns[lastMatchedIndex].negated &&
|
||||
lastMatchedIndex > lastPositiveIndex
|
||||
}
|
||||
}
|
||||
|
||||
export function isIncludedByWorktreePatterns(
|
||||
patterns: readonly WorktreeIncludePattern[],
|
||||
relativePath: string,
|
||||
@@ -132,15 +171,6 @@ export function isIncludedByWorktreePatterns(
|
||||
budget: WorktreeIncludeMatchBudget,
|
||||
ignoreCase: boolean = false
|
||||
): boolean {
|
||||
// Why: gitignore patterns are order-sensitive, so negations must use last-match-wins.
|
||||
let included = false
|
||||
const caseFoldedRelativePath = ignoreCase ? relativePath.toLowerCase() : relativePath
|
||||
for (const pattern of patterns) {
|
||||
if (
|
||||
patternMatches(pattern, relativePath, caseFoldedRelativePath, isDirectory, budget, ignoreCase)
|
||||
) {
|
||||
included = !pattern.negated
|
||||
}
|
||||
}
|
||||
return included
|
||||
return getWorktreeIncludePatternDecision(patterns, relativePath, isDirectory, budget, ignoreCase)
|
||||
.included
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { execFile } from 'node:child_process'
|
||||
import { execFile, type ExecFileOptions } from 'node:child_process'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { mkdir, stat, lstat, rm, link, rmdir, chmod } from 'node:fs/promises'
|
||||
import { dirname, resolve, sep } from 'node:path'
|
||||
@@ -6,10 +6,12 @@ import { promisify } from 'node:util'
|
||||
|
||||
type ExecFileAsync = (
|
||||
file: string,
|
||||
args: readonly string[]
|
||||
args: readonly string[],
|
||||
options?: Pick<ExecFileOptions, 'timeout'>
|
||||
) => Promise<{ stdout: string; stderr: string }>
|
||||
|
||||
const execFileAsync = promisify(execFile) as ExecFileAsync
|
||||
const APFS_FILESYSTEM_PROBE_TIMEOUT_MS = 5_000
|
||||
|
||||
export type ApfsCloneDeps = {
|
||||
execFileAsync: ExecFileAsync
|
||||
@@ -55,16 +57,18 @@ async function getDarwinFilesystemInfo(
|
||||
path: string,
|
||||
deps: ApfsCloneDeps
|
||||
): Promise<DarwinFilesystemInfo> {
|
||||
const { stdout: dfOutput } = await deps.execFileAsync('/bin/df', ['-P', path])
|
||||
const { stdout: dfOutput } = await deps.execFileAsync('/bin/df', ['-P', path], {
|
||||
timeout: APFS_FILESYSTEM_PROBE_TIMEOUT_MS
|
||||
})
|
||||
const device = dfOutput.trim().split(/\r?\n/)[1]?.trim().split(/\s+/)[0]
|
||||
if (!device) {
|
||||
throw new Error(`Could not resolve filesystem device for ${path}`)
|
||||
}
|
||||
const { stdout: diskutilOutput } = await deps.execFileAsync('/usr/sbin/diskutil', [
|
||||
'info',
|
||||
'-plist',
|
||||
device
|
||||
])
|
||||
const { stdout: diskutilOutput } = await deps.execFileAsync(
|
||||
'/usr/sbin/diskutil',
|
||||
['info', '-plist', device],
|
||||
{ timeout: APFS_FILESYSTEM_PROBE_TIMEOUT_MS }
|
||||
)
|
||||
const filesystemNameMatch = /<key>FilesystemName<\/key>\s*<string>([^<]+)<\/string>/u.exec(
|
||||
diskutilOutput
|
||||
)
|
||||
|
||||
@@ -617,6 +617,9 @@ describe('createWorktreeCopiedPaths', () => {
|
||||
([file]) => file === '/bin/df' || file === '/usr/sbin/diskutil'
|
||||
)
|
||||
expect(filesystemProbeCalls).toHaveLength(2)
|
||||
expect(
|
||||
filesystemProbeCalls.every(([, , execOptions]) => execOptions?.timeout === 5_000)
|
||||
).toBe(true)
|
||||
const copyCalls = execFileAsyncMock.mock.calls.filter(([file]) => file === '/bin/cp')
|
||||
expect(copyCalls).toHaveLength(diskutilError ? 0 : 2)
|
||||
expect(copyCalls.every(([, args]) => !args.includes('-L'))).toBe(true)
|
||||
|
||||
Reference in New Issue
Block a user