diff --git a/src/main/ipc/worktree-apfs-clone.ts b/src/main/ipc/worktree-apfs-clone.ts index 69587b88de1..2d3b13573ab 100644 --- a/src/main/ipc/worktree-apfs-clone.ts +++ b/src/main/ipc/worktree-apfs-clone.ts @@ -29,6 +29,12 @@ type DarwinFilesystemInfo = { filesystemName: string } +/** Per-materialization cache keyed by `stat().dev`. Copying N `.worktreeinclude` + * paths would otherwise re-run df+diskutil per path (4 subprocesses each) even + * though source and worktree almost always share one volume; caching collapses + * that to one probe per distinct volume. */ +export type DarwinFilesystemCache = Map> + export class ApfsCloneUnavailableError extends Error { constructor(message: string) { super(message) @@ -72,14 +78,32 @@ async function getDarwinFilesystemInfo( } } +async function getCachedDarwinFilesystemInfo( + path: string, + deps: ApfsCloneDeps, + cache: DarwinFilesystemCache +): Promise { + const deviceId = (await stat(path)).dev + const cached = cache.get(deviceId) + if (cached) { + return cached + } + // Why: cache the pending (or rejected) probe so every path on this volume + // reuses one df+diskutil pair instead of respawning them per copy. + const pending = getDarwinFilesystemInfo(path, deps) + cache.set(deviceId, pending) + return pending +} + async function assertSameApfsVolume( source: string, target: string, - deps: ApfsCloneDeps + deps: ApfsCloneDeps, + cache: DarwinFilesystemCache ): 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 || @@ -151,10 +175,11 @@ export async function cloneWorktreePathWithApfs( source: string, target: string, sourceIsDirectory: boolean, - deps: ApfsCloneDeps = defaultApfsCloneDeps + deps: ApfsCloneDeps = defaultApfsCloneDeps, + filesystemCache: DarwinFilesystemCache = new Map() ): Promise { await mkdir(dirname(target), { recursive: true }) - await assertSameApfsVolume(source, target, deps) + await assertSameApfsVolume(source, target, deps, filesystemCache) // 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. diff --git a/src/main/ipc/worktree-symlinks.test.ts b/src/main/ipc/worktree-symlinks.test.ts index 148404ebe4c..865c3f17b11 100644 --- a/src/main/ipc/worktree-symlinks.test.ts +++ b/src/main/ipc/worktree-symlinks.test.ts @@ -447,6 +447,31 @@ describe('createWorktreeCopiedPaths', () => { expect(readFileSync(join(worktree, 'apps', 'web', '.env'), 'utf8')).toBe('A=1') }) + // Finding 1 regression: a symlinked include entry must become an independent + // copy, not a symlink, or worktree edits would leak back into the shared target. + posixIt('dereferences a symlinked file entry so edits do not leak to the primary', async () => { + writeFileSync(join(primary, '.env.shared'), 'SECRET=1\n') + symlinkSync(join(primary, '.env.shared'), join(primary, '.env')) + + await createWorktreeCopiedPaths(primary, worktree, ['.env'], { platform: 'linux' }) + + expect(lstatSync(join(worktree, '.env')).isSymbolicLink()).toBe(false) + writeFileSync(join(worktree, '.env'), 'SECRET=2\n') + expect(readFileSync(join(primary, '.env.shared'), 'utf8')).toBe('SECRET=1\n') + }) + + posixIt('dereferences a symlinked directory entry into an independent copy', async () => { + mkdirSync(join(primary, '.cache-real')) + writeFileSync(join(primary, '.cache-real', 'f'), 'ORIG\n') + symlinkSync(join(primary, '.cache-real'), join(primary, '.cache'), 'dir') + + await createWorktreeCopiedPaths(primary, worktree, ['.cache'], { platform: 'linux' }) + + expect(lstatSync(join(worktree, '.cache')).isSymbolicLink()).toBe(false) + writeFileSync(join(worktree, '.cache', 'f'), 'CHANGED\n') + expect(readFileSync(join(primary, '.cache-real', 'f'), 'utf8')).toBe('ORIG\n') + }) + it('preserves a pre-existing target in the worktree (no clobber)', async () => { writeFileSync(join(primary, '.env'), 'SECRET=1\n') writeFileSync(join(worktree, '.env'), 'MINE=1\n') @@ -502,6 +527,34 @@ describe('createWorktreeCopiedPaths', () => { ) expect(readFileSync(join(worktree, '.env'), 'utf8')).toBe('CLONED=1\n') }) + + // Perf: the df+diskutil volume probe must not scale with the number of copied + // paths — one probe per distinct volume, cached across the materialization. + it('probes each APFS volume once regardless of how many paths are copied', async () => { + for (const name of ['.env', '.env.local', 'config.json', 'secrets.json']) { + writeFileSync(join(primary, name), `${name}\n`) + } + const deps = createApfsCloneDeps({ onCp: () => {} }) + + await createWorktreeCopiedPaths( + primary, + worktree, + ['.env', '.env.local', 'config.json', 'secrets.json'], + { platform: 'darwin', apfsCloneDeps: deps } + ) + + const execFileAsyncMock = vi.mocked(deps.execFileAsync) + const dfCalls = execFileAsyncMock.mock.calls.filter(([file]) => file === '/bin/df').length + const diskutilCalls = execFileAsyncMock.mock.calls.filter( + ([file]) => file === '/usr/sbin/diskutil' + ).length + // 4 paths would be 8 df + 8 diskutil un-cached; source+worktree share one + // tmp volume, so caching collapses this to a single probe pair. + expect(dfCalls).toBeLessThanOrEqual(2) + expect(diskutilCalls).toBeLessThanOrEqual(2) + // The copies themselves still happen per path. + expect(execFileAsyncMock.mock.calls.filter(([file]) => file === '/bin/cp')).toHaveLength(4) + }) }) describe('removeWorktreeSymlinks', () => { diff --git a/src/main/ipc/worktree-symlinks.ts b/src/main/ipc/worktree-symlinks.ts index af6307afc8a..fb81cc37df4 100644 --- a/src/main/ipc/worktree-symlinks.ts +++ b/src/main/ipc/worktree-symlinks.ts @@ -1,11 +1,12 @@ -import { symlink, mkdir, stat, lstat, unlink, cp } from 'node:fs/promises' +import { symlink, mkdir, stat, lstat, unlink, cp, realpath } from 'node:fs/promises' import { dirname, isAbsolute, resolve } from 'node:path' import { ApfsCloneUnavailableError, cloneWorktreePathWithApfs, defaultApfsCloneDeps, WorktreeLinkedPathTargetExistsError, - type ApfsCloneDeps + type ApfsCloneDeps, + type DarwinFilesystemCache } from './worktree-apfs-clone' type WorktreeLinkedPathOptions = { @@ -69,9 +70,15 @@ async function createWorktreeLinkedPath( sourceIsDirectory: boolean, sourceIsSymbolicLink: boolean, mode: WorktreeMaterializeMode, - options: WorktreeLinkedPathOptions + options: WorktreeLinkedPathOptions, + apfsFilesystemCache: DarwinFilesystemCache ): Promise { - if (options.platform === 'darwin' && !sourceIsSymbolicLink) { + // Why: copy mode promises each worktree an independent copy; copying the + // symlink itself would recreate a link to the shared target, so edits in the + // worktree would leak back into the primary checkout (or escape it entirely if + // the link points outside). Resolve the real source so we copy content. + const copySource = mode === 'copy' && sourceIsSymbolicLink ? await realpath(source) : source + if (options.platform === 'darwin' && (!sourceIsSymbolicLink || mode === 'copy')) { try { const cloneWorktreePath = options.cloneWorktreePath ?? @@ -80,9 +87,10 @@ async function createWorktreeLinkedPath( cloneSource, cloneTarget, cloneSourceIsDirectory, - options.apfsCloneDeps ?? defaultApfsCloneDeps + options.apfsCloneDeps ?? defaultApfsCloneDeps, + apfsFilesystemCache )) - await cloneWorktreePath(source, target, sourceIsDirectory) + await cloneWorktreePath(copySource, target, sourceIsDirectory) return } catch (error) { if (error instanceof WorktreeLinkedPathTargetExistsError) { @@ -97,7 +105,7 @@ async function createWorktreeLinkedPath( } } if (mode === 'copy') { - await copyWorktreePath(source, target) + await copyWorktreePath(copySource, target) return } await symlinkWorktreePath(source, target, sourceIsDirectory) @@ -122,6 +130,9 @@ async function materializeWorktreePaths( options: WorktreeLinkedPathOptions = {} ): Promise { const effectiveOptions = { platform: process.platform, ...options } + // Why: one df+diskutil probe per distinct volume for the whole materialization, + // not per copied path — see DarwinFilesystemCache. + const apfsFilesystemCache: DarwinFilesystemCache = new Map() for (const rawPath of paths) { const safePath = getSafeRelativePath(rawPath) @@ -159,7 +170,8 @@ async function materializeWorktreePaths( sourceIsDirectory, sourceIsSymbolicLink, mode, - effectiveOptions + effectiveOptions, + apfsFilesystemCache ) } catch (error) { console.error(