diff --git a/src/main/git/remove-worktree.test.ts b/src/main/git/remove-worktree.test.ts index db008799de9..15e115221ea 100644 --- a/src/main/git/remove-worktree.test.ts +++ b/src/main/git/remove-worktree.test.ts @@ -9,14 +9,26 @@ const { translateWslOutputPathsMock, statMock, readFileMock, - resolveGitDirMock + resolveGitDirMock, + moveWorktreeDirectoryToTrashMock, + restoreWorktreeDirectoryFromTrashMock, + scheduleWorktreeTrashDeletionMock } = vi.hoisted(() => ({ gitExecFileAsyncMock: vi.fn(), gitExecFileSyncMock: vi.fn(), translateWslOutputPathsMock: vi.fn((output: string) => output), statMock: vi.fn(), readFileMock: vi.fn(), - resolveGitDirMock: vi.fn() + resolveGitDirMock: vi.fn(), + moveWorktreeDirectoryToTrashMock: vi.fn(), + restoreWorktreeDirectoryFromTrashMock: vi.fn(), + scheduleWorktreeTrashDeletionMock: vi.fn() +})) + +vi.mock('../worktree-trash', () => ({ + moveWorktreeDirectoryToTrash: moveWorktreeDirectoryToTrashMock, + restoreWorktreeDirectoryFromTrash: restoreWorktreeDirectoryFromTrashMock, + scheduleWorktreeTrashDeletion: scheduleWorktreeTrashDeletionMock })) vi.mock('./runner', () => ({ @@ -45,7 +57,8 @@ import { removeWorktree, _resetWorktreeScanCacheForTests, WORKTREE_LIST_TIMEOUT_MS, - WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS + WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS, + WORKTREE_REMOVAL_REGISTRATION_TIMEOUT_MS } from './worktree' // Why: detectSparseCheckout on main also requires core.sparseCheckout=true in git @@ -57,6 +70,12 @@ const ENABLED_SPARSE_CHECKOUT_CONFIG = '[core]\nsparseCheckout = true\n' beforeEach(() => { clearGitCapabilityStateForTests() _resetWorktreeScanCacheForTests() + // Default: the checkout cannot be renamed aside, so removal deletes it in place. + moveWorktreeDirectoryToTrashMock.mockReset() + moveWorktreeDirectoryToTrashMock.mockResolvedValue(undefined) + restoreWorktreeDirectoryFromTrashMock.mockReset() + restoreWorktreeDirectoryFromTrashMock.mockResolvedValue(true) + scheduleWorktreeTrashDeletionMock.mockReset() }) type MockResult = { @@ -276,6 +295,8 @@ branch refs/heads/main const calls = getGitCalls() expect(calls).toEqual([ 'git worktree list --porcelain -z', + // The cleanliness probe that decides whether the checkout may be renamed aside. + 'git status --porcelain --untracked-files=all', 'git worktree remove /repo-feature', 'git branch -d -- feature/test', 'git worktree prune', @@ -283,6 +304,209 @@ branch refs/heads/main ]) }) + it('renames the checkout aside and deregisters the missing path', async () => { + mockGitCommands({ + 'git worktree list --porcelain': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main + +worktree /repo-feature +HEAD def456 +branch refs/heads/feature/test +` + }, + 'git worktree list --porcelain#2': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main +` + } + }) + moveWorktreeDirectoryToTrashMock.mockResolvedValue('/trash/wt-1-abcdef01') + + await removeWorktree('/repo', '/repo-feature') + + const calls = getGitCalls() + expect(moveWorktreeDirectoryToTrashMock).toHaveBeenCalledWith('/repo-feature') + expect(scheduleWorktreeTrashDeletionMock).toHaveBeenCalledWith('/trash/wt-1-abcdef01') + expect(calls).toContain('git worktree remove --force /repo-feature') + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['worktree', 'remove', '--force', '/repo-feature'], + { cwd: '/repo', timeout: WORKTREE_REMOVAL_REGISTRATION_TIMEOUT_MS } + ) + expect(calls).not.toContain('git worktree remove /repo-feature') + expect(calls).not.toContain('git worktree prune') + expect(calls).toContain('git branch -d -- feature/test') + }) + + it('prunes the registration when deregistering the moved checkout fails', async () => { + mockGitCommands({ + 'git worktree list --porcelain': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main + +worktree /repo-feature +HEAD def456 +branch refs/heads/feature/test +` + }, + 'git worktree list --porcelain#2': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main +` + }, + 'git worktree remove --force /repo-feature': { + error: new Error('fatal: validation failed, cannot remove working directory') + } + }) + moveWorktreeDirectoryToTrashMock.mockResolvedValue('/trash/wt-2-abcdef02') + + await removeWorktree('/repo', '/repo-feature') + + expect(getGitCalls()).toContain('git worktree prune') + expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], { + cwd: '/repo', + timeout: WORKTREE_REMOVAL_REGISTRATION_TIMEOUT_MS + }) + expect( + gitExecFileAsyncMock.mock.calls.filter( + ([args, options]) => + args.join(' ') === 'worktree list --porcelain -z' && + options.timeout === WORKTREE_REMOVAL_REGISTRATION_TIMEOUT_MS + ) + ).toHaveLength(2) + expect(scheduleWorktreeTrashDeletionMock).toHaveBeenCalledWith('/trash/wt-2-abcdef02') + expect(restoreWorktreeDirectoryFromTrashMock).not.toHaveBeenCalled() + }) + + it('restores the moved checkout and removes in place when the registration survives pruning', async () => { + mockGitCommands({ + 'git worktree list --porcelain': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main + +worktree /repo-feature +HEAD def456 +branch refs/heads/feature/test +` + }, + 'git worktree remove --force /repo-feature': { + error: new Error('fatal: validation failed, cannot remove working directory') + } + }) + moveWorktreeDirectoryToTrashMock.mockResolvedValue('/trash/wt-3-abcdef03') + + await removeWorktree('/repo', '/repo-feature') + + expect(restoreWorktreeDirectoryFromTrashMock).toHaveBeenCalledWith( + '/trash/wt-3-abcdef03', + '/repo-feature' + ) + expect(scheduleWorktreeTrashDeletionMock).not.toHaveBeenCalled() + expect(getGitCalls()).toContain('git worktree remove /repo-feature') + }) + + it('removes in place when the checkout cannot be renamed aside', async () => { + mockGitCommands({ + 'git worktree list --porcelain': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main + +worktree /repo-feature +HEAD def456 +branch refs/heads/feature/test +` + }, + 'git worktree list --porcelain#2': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main +` + } + }) + // Windows open handles and cross-volume renames both surface as an unavailable rename. + moveWorktreeDirectoryToTrashMock.mockResolvedValue(undefined) + + await removeWorktree('/repo', '/repo-feature') + + expect(getGitCalls()).toContain('git worktree remove /repo-feature') + expect(scheduleWorktreeTrashDeletionMock).not.toHaveBeenCalled() + }) + + it('never renames a dirty checkout aside', async () => { + mockGitCommands({ + 'git worktree list --porcelain': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main + +worktree /repo-feature +HEAD def456 +branch refs/heads/feature/test +` + }, + 'git status --porcelain --untracked-files=all': { stdout: ' M src/app.ts\n' }, + 'git worktree remove /repo-feature': { + error: new Error('fatal: contains modified or untracked files, use --force to delete it') + } + }) + + await expect(removeWorktree('/repo', '/repo-feature')).rejects.toThrow( + 'contains modified or untracked files' + ) + expect(moveWorktreeDirectoryToTrashMock).not.toHaveBeenCalled() + }) + + it('deletes WSL-hosted checkouts in place inside the distro', async () => { + mockGitCommands({ + 'git worktree list --porcelain': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main + +worktree /repo-feature +HEAD def456 +branch refs/heads/feature/test +` + }, + 'git worktree list --porcelain#2': { + stdout: `worktree /repo +HEAD abc123 +branch refs/heads/main +` + } + }) + + await removeWorktree('/repo', '/repo-feature', false, { wslDistro: 'Ubuntu' }) + + expect(moveWorktreeDirectoryToTrashMock).not.toHaveBeenCalled() + expect(getGitCalls()).not.toContain('git status --porcelain --untracked-files=all') + expect(getGitCalls()).toContain('git worktree remove /repo-feature') + }) + + it('does not rename a WSL checkout configured for a native Windows repo', async () => { + const originalPlatform = process.platform + const worktreePath = '\\\\wsl.localhost\\Ubuntu\\home\\dev\\feature' + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + mockGitCommands({}) + + await removeWorktree('C:\\repo', worktreePath, false, { + knownRemovedWorktree: { branch: '', head: '', locked: false } + }) + + expect(moveWorktreeDirectoryToTrashMock).not.toHaveBeenCalled() + expect(getGitCalls()).toEqual([`git worktree remove ${worktreePath}`]) + } finally { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + } + }) + it('passes one --force before the worktree path for dirty-file removal', async () => { mockGitCommands({ 'git worktree list --porcelain': { @@ -339,13 +563,15 @@ branch refs/heads/main expectGitCallOrder( calls, 'git worktree remove /repo-feature', - 'git status --porcelain --untracked-files=all' - ) - expectGitCallOrder( - calls, - 'git status --porcelain --untracked-files=all', 'git worktree remove --force /repo-feature' ) + // The re-proof of cleanliness between the refusal and the forced retry. + expect(calls.lastIndexOf('git status --porcelain --untracked-files=all')).toBeGreaterThan( + calls.indexOf('git worktree remove /repo-feature') + ) + expect(calls.lastIndexOf('git status --porcelain --untracked-files=all')).toBeLessThan( + calls.indexOf('git worktree remove --force /repo-feature') + ) expect(calls).toContain('git branch -d -- feature/test') }) @@ -420,7 +646,10 @@ branch refs/heads/feature/test 'git worktree remove failed' ) expect(getGitCalls()).not.toContain('git worktree remove --force /repo-feature') - expect(getGitCalls()).not.toContain('git status --porcelain --untracked-files=all') + // Only the pre-rename probe ran: an unrelated failure must not re-prove cleanliness. + expect( + getGitCalls().filter((call) => call === 'git status --porcelain --untracked-files=all') + ).toHaveLength(1) }) it('rejects a locked worktree with stable app-owned copy before invoking remove', async () => { diff --git a/src/main/git/worktree-deferred-removal-real-git.test.ts b/src/main/git/worktree-deferred-removal-real-git.test.ts new file mode 100644 index 00000000000..374be4ecc0f --- /dev/null +++ b/src/main/git/worktree-deferred-removal-real-git.test.ts @@ -0,0 +1,113 @@ +// Real-binary coverage for deferred worktree deletion: the mocked-runner suite cannot prove that Git +// accepts `worktree remove --force` on a path Orca just renamed away. +import { execFile } from 'node:child_process' +import { existsSync } from 'node:fs' +import { mkdir, mkdtemp, readdir, realpath, 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 { removeWorktree } from './worktree' +import { + getWorktreeTrashRoot, + isWorktreeTrashEntryName, + sweepStaleWorktreeTrash, + whenWorktreeTrashDeletionsSettled, + WORKTREE_TRASH_DIR_NAME +} from '../worktree-trash' + +const execFileAsync = promisify(execFile) + +let scratchDir = '' +let repoPath = '' +let workspaceRoot = '' +let worktreePath = '' + +async function git(args: string[], cwd: string): Promise { + const { stdout } = await execFileAsync('git', args, { cwd }) + return stdout +} + +beforeEach(async () => { + // realpath: macOS hands out /var/... temp paths while Git reports /private/var/..., and Orca + // matches the worktree it is removing against Git's own list. + scratchDir = await realpath(await mkdtemp(join(tmpdir(), 'orca-deferred-worktree-removal-'))) + repoPath = join(scratchDir, 'repo') + workspaceRoot = join(scratchDir, 'workspaces') + worktreePath = join(workspaceRoot, 'repo', 'feature') + await mkdir(repoPath, { recursive: true }) + await mkdir(join(workspaceRoot, 'repo'), { recursive: true }) + await git(['init', '-q'], repoPath) + await git(['config', 'user.email', 'deferred@example.invalid'], repoPath) + await git(['config', 'user.name', 'Deferred Removal'], repoPath) + await writeFile(join(repoPath, 'seed.txt'), 'seed\n') + // Committed before the worktree exists so its branch stays merged and branch cleanup can run. + await mkdir(join(repoPath, 'node_modules', 'pkg'), { recursive: true }) + await writeFile(join(repoPath, 'node_modules', 'pkg', 'index.js'), 'module.exports = 1\n') + await git(['add', '-A'], repoPath) + await git(['commit', '-qm', 'seed'], repoPath) + await git(['worktree', 'add', '-q', worktreePath, '-b', 'feature'], repoPath) +}) + +afterEach(async () => { + await whenWorktreeTrashDeletionsSettled() + await rm(scratchDir, { recursive: true, force: true }) +}) + +describe('deferred worktree removal against the real Git binary', () => { + it('clears the registration and deletes the renamed checkout in the background', async () => { + const trashRoot = getWorktreeTrashRoot(worktreePath) + + await removeWorktree(repoPath, worktreePath, false, { deleteBranch: false }) + + // The user-visible removal is complete: nothing on disk, nothing registered. + expect(existsSync(worktreePath)).toBe(false) + expect(await git(['worktree', 'list'], repoPath)).not.toContain(worktreePath) + // Only the rename path creates this root, so its presence proves the deletion was deferred. + expect(existsSync(trashRoot)).toBe(true) + expect((await readdir(trashRoot)).every(isWorktreeTrashEntryName)).toBe(true) + + await whenWorktreeTrashDeletionsSettled() + expect(await readdir(trashRoot)).toEqual([]) + }) + + it('leaves sibling worktrees registered', async () => { + const siblingPath = join(workspaceRoot, 'repo', 'sibling') + await git(['worktree', 'add', '-q', siblingPath, '-b', 'sibling'], repoPath) + + await removeWorktree(repoPath, worktreePath, false, { deleteBranch: false }) + + expect(await git(['worktree', 'list'], repoPath)).toContain(siblingPath) + expect(existsSync(siblingPath)).toBe(true) + }) + + it('deletes the branch exactly as the in-place removal did', async () => { + await removeWorktree(repoPath, worktreePath, false) + + expect(await git(['branch', '--list', 'feature'], repoPath)).toBe('') + }) + + it('refuses to move a dirty checkout aside', async () => { + await writeFile(join(worktreePath, 'seed.txt'), 'edited\n') + + await expect(removeWorktree(repoPath, worktreePath, false)).rejects.toThrow() + expect(existsSync(join(worktreePath, 'seed.txt'))).toBe(true) + expect(await git(['worktree', 'list'], repoPath)).toContain(worktreePath) + expect(existsSync(getWorktreeTrashRoot(worktreePath))).toBe(false) + }) + + it('sweeps trash a previous run left behind', async () => { + const stalePath = join( + workspaceRoot, + 'repo', + WORKTREE_TRASH_DIR_NAME, + 'wt-1700000000000-deadbeef' + ) + await mkdir(join(stalePath, 'node_modules'), { recursive: true }) + + await sweepStaleWorktreeTrash([workspaceRoot]) + + expect(existsSync(stalePath)).toBe(false) + expect(existsSync(worktreePath)).toBe(true) + }) +}) diff --git a/src/main/git/worktree-removal-large-tree.bench.test.ts b/src/main/git/worktree-removal-large-tree.bench.test.ts new file mode 100644 index 00000000000..debeb6210ed --- /dev/null +++ b/src/main/git/worktree-removal-large-tree.bench.test.ts @@ -0,0 +1,85 @@ +// Manual benchmark for the `worktree.remove.git_remove` stage on a large checkout. +// Opt in (it builds ~100k files): ORCA_WORKTREE_REMOVAL_BENCH=1 pnpm exec vitest run \ +// --config config/vitest.config.ts src/main/git/worktree-removal-large-tree.bench.test.ts +import { execFile } from 'node:child_process' +import { mkdir, mkdtemp, readdir, rm, writeFile } from 'node:fs/promises' +import { existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { promisify } from 'node:util' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { removeWorktree } from './worktree' +import { getWorktreeTrashRoot, whenWorktreeTrashDeletionsSettled } from '../worktree-trash' + +const execFileAsync = promisify(execFile) +const describeBench = process.env.ORCA_WORKTREE_REMOVAL_BENCH ? describe : describe.skip + +const FIXTURE_DIRECTORIES = 200 +const FIXTURE_FILES_PER_DIRECTORY = 500 + +describeBench('worktree removal on a large checkout', () => { + let scratchDir = '' + let repoPath = '' + let worktreePath = '' + + async function git(args: string[], cwd: string): Promise { + await execFileAsync('git', args, { cwd, maxBuffer: 64 * 1024 * 1024 }) + } + + beforeAll(async () => { + scratchDir = await mkdtemp(join(tmpdir(), 'orca-worktree-removal-bench-')) + repoPath = join(scratchDir, 'repo') + worktreePath = join(scratchDir, 'workspaces', 'repo', 'bench') + await mkdir(repoPath, { recursive: true }) + await git(['init', '-q', '-b', 'main'], repoPath) + await git(['config', 'user.email', 'bench@example.invalid'], repoPath) + await git(['config', 'user.name', 'Bench'], repoPath) + await writeFile(join(repoPath, 'seed.txt'), 'seed\n') + await git(['add', 'seed.txt'], repoPath) + await git(['commit', '-qm', 'seed'], repoPath) + await mkdir(join(scratchDir, 'workspaces', 'repo'), { recursive: true }) + await git(['worktree', 'add', '-q', worktreePath, '-b', 'bench'], repoPath) + + // A committed synthetic node_modules-shaped tree: the removal must be clean-checked and deleted. + for (let dirIndex = 0; dirIndex < FIXTURE_DIRECTORIES; dirIndex += 1) { + const dir = join(worktreePath, 'node_modules', `pkg-${dirIndex}`) + await mkdir(dir, { recursive: true }) + await Promise.all( + Array.from({ length: FIXTURE_FILES_PER_DIRECTORY }, (_unused, fileIndex) => + writeFile(join(dir, `file-${fileIndex}.js`), `module.exports = ${fileIndex}\n`) + ) + ) + } + await git(['add', '-A'], worktreePath) + await git(['commit', '-qm', 'large tree'], worktreePath) + }, 900_000) + + afterAll(async () => { + await whenWorktreeTrashDeletionsSettled() + if (scratchDir) { + await rm(scratchDir, { recursive: true, force: true }) + } + }) + + it('returns from removeWorktree without waiting for the recursive delete', async () => { + const startedAt = Date.now() + await removeWorktree(repoPath, worktreePath, false, { deleteBranch: false }) + const userVisibleMs = Date.now() - startedAt + + const trashRoot = getWorktreeTrashRoot(worktreePath) + console.log( + `[bench] files=${FIXTURE_DIRECTORIES * FIXTURE_FILES_PER_DIRECTORY} removeWorktree=${userVisibleMs}ms` + ) + expect(existsSync(worktreePath)).toBe(false) + const { stdout } = await execFileAsync('git', ['worktree', 'list'], { cwd: repoPath }) + expect(stdout).not.toContain(worktreePath) + + const backgroundStartedAt = Date.now() + await whenWorktreeTrashDeletionsSettled() + console.log( + `[bench] background deletion=${Date.now() - backgroundStartedAt}ms (trash root ${trashRoot})` + ) + expect(existsSync(trashRoot)).toBe(true) + expect(await readdir(trashRoot)).toEqual([]) + }, 900_000) +}) diff --git a/src/main/git/worktree.test.ts b/src/main/git/worktree.test.ts index 493beb1d91f..ff72ce6ea2c 100644 --- a/src/main/git/worktree.test.ts +++ b/src/main/git/worktree.test.ts @@ -4,13 +4,17 @@ without a meaningful boundary. */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { gitExecFileAsyncMock, gitExecFileSyncMock, translateWslOutputPathsMock } = vi.hoisted( - () => ({ - gitExecFileAsyncMock: vi.fn(), - gitExecFileSyncMock: vi.fn(), - translateWslOutputPathsMock: vi.fn((output: string) => output) - }) -) +const { + gitExecFileAsyncMock, + gitExecFileSyncMock, + translateWslOutputPathsMock, + moveWorktreeDirectoryToTrashMock +} = vi.hoisted(() => ({ + gitExecFileAsyncMock: vi.fn(), + gitExecFileSyncMock: vi.fn(), + translateWslOutputPathsMock: vi.fn((output: string) => output), + moveWorktreeDirectoryToTrashMock: vi.fn() +})) vi.mock('./runner', () => ({ gitExecFileAsync: gitExecFileAsyncMock, @@ -18,6 +22,13 @@ vi.mock('./runner', () => ({ translateWslOutputPaths: translateWslOutputPathsMock })) +// Default: the checkout cannot be renamed aside, so removal deletes it in place. +vi.mock('../worktree-trash', () => ({ + moveWorktreeDirectoryToTrash: moveWorktreeDirectoryToTrashMock.mockResolvedValue(undefined), + restoreWorktreeDirectoryFromTrash: vi.fn().mockResolvedValue(true), + scheduleWorktreeTrashDeletion: vi.fn() +})) + import { clearGitCapabilityStateForTests } from './git-capability-state' import { @@ -1792,6 +1803,7 @@ describe('removeWorktree', () => { it('uses safe `branch -d` and preserves a branch with unmerged commits', async () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: beforeRemoval }) // list before + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // clean probe before the rename attempt gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree remove // Git refuses to delete an unmerged branch with `-d`. gitExecFileAsyncMock.mockRejectedValueOnce(new Error('not fully merged')) // branch -d @@ -1808,6 +1820,7 @@ describe('removeWorktree', () => { it('deletes the branch when `branch -d` succeeds (fully merged)', async () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: beforeRemoval }) // list before + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // clean probe before the rename attempt gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree remove gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // branch -d succeeds @@ -1822,6 +1835,7 @@ describe('removeWorktree', () => { }) it('reuses known removed worktree metadata instead of relisting before removal', async () => { + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // clean probe before the rename attempt gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree remove gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // branch -d succeeds @@ -1833,6 +1847,7 @@ describe('removeWorktree', () => { }) expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0])).toEqual([ + ['status', '--porcelain', '--untracked-files=all'], ['worktree', 'remove', '/repo-feature'], ['branch', '-d', '--', 'feature/test'] ]) @@ -1840,6 +1855,7 @@ describe('removeWorktree', () => { it('prunes and retries branch deletion only when Git reports a checked-out branch', async () => { gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: beforeRemoval }) // list before + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // clean probe before the rename attempt gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '' }) // worktree remove gitExecFileAsyncMock.mockRejectedValueOnce( new Error("error: cannot delete branch 'feature/test' used by worktree at '/repo-stale'") @@ -1851,6 +1867,7 @@ describe('removeWorktree', () => { expect(gitExecFileAsyncMock.mock.calls.map((call) => call[0])).toEqual([ ['worktree', 'list', '--porcelain', '-z'], + ['status', '--porcelain', '--untracked-files=all'], ['worktree', 'remove', '/repo-feature'], ['branch', '-d', '--', 'feature/test'], ['worktree', 'prune'], diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index 4ea08ef687b..064eb0887fd 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -7,6 +7,13 @@ import { } from '../../shared/git-branch-cleanup' import { resolveWorktreeAddBaseRef } from '../../shared/worktree-base-ref' import { withSpan } from '../observability/tracer' +import { withWorktreeRemoveStageSpan } from '../observability/instrumentation' +import { + moveWorktreeDirectoryToTrash, + restoreWorktreeDirectoryFromTrash, + scheduleWorktreeTrashDeletion +} from '../worktree-trash' +import { parseWslPath } from '../wsl' import type { GitWorktreeInfo, LocalBaseRefRefreshResult, @@ -87,6 +94,7 @@ const PRUNABLE_EXISTENCE_PROBE_CONCURRENCY = 8 // Why: bound `git worktree add` so a OneDrive cloud-placeholder stall fails fast (STA-1292); generous enough for a legit large checkout (#7225). export const WORKTREE_ADD_TIMEOUT_MS = 180_000 export const WORKTREE_REMOVAL_PREFLIGHT_TIMEOUT_MS = 30_000 +export const WORKTREE_REMOVAL_REGISTRATION_TIMEOUT_MS = 30_000 // Why: one wedged shared scan otherwise hangs every later list, including create's post-add re-list. export const WORKTREE_LIST_TIMEOUT_MS = 30_000 @@ -1131,23 +1139,27 @@ async function performRemoveWorktree( // Why: callers outside the IPC/runtime preflight must not bypass Git's lock contract or rely on localized stderr after side effects. assertWorktreeUnlockedForRemoval(removedWorktree) - const args = ['worktree', 'remove'] - if (force) { - args.push('--force') - } - args.push(worktreePath) - try { - await gitExecFileAsync(args, gitExecOptions(repoPath, options)) - } catch (error) { - if (force || !isSubmoduleWorktreeRemovalRefusal(error)) { - throw error + if ( + !(await tryRemoveWorktreeWithDeferredDirectoryDeletion(repoPath, worktreePath, force, options)) + ) { + const args = ['worktree', 'remove'] + if (force) { + args.push('--force') + } + args.push(worktreePath) + try { + await gitExecFileAsync(args, gitExecOptions(repoPath, options)) + } catch (error) { + if (force || !isSubmoduleWorktreeRemovalRefusal(error)) { + throw error + } + // Why: Git refuses non-force removal of a worktree with an initialised submodule even when clean; re-prove cleanliness, then --force. + await assertWorktreeCleanForRemoval(worktreePath, false, options) + await gitExecFileAsync( + ['worktree', 'remove', '--force', worktreePath], + gitExecOptions(repoPath, options) + ) } - // Why: Git refuses non-force removal of a worktree with an initialised submodule even when clean; re-prove cleanliness, then --force. - await assertWorktreeCleanForRemoval(worktreePath, false, options) - await gitExecFileAsync( - ['worktree', 'remove', '--force', worktreePath], - gitExecOptions(repoPath, options) - ) } if (!branchName) { @@ -1164,6 +1176,82 @@ async function performRemoveWorktree( ) } +/** + * Rename the checkout into a sibling trash directory and clear Git's registration for the + * now-missing path, so the multi-GB recursive delete runs after this removal has returned. + * Returns false when the caller must let `git worktree remove` delete the directory inline. + */ +async function tryRemoveWorktreeWithDeferredDirectoryDeletion( + repoPath: string, + worktreePath: string, + force: boolean, + options: RemoveWorktreeOptions +): Promise { + // Why: WSL-owned checkouts are deleted inside the distro, so Node on Windows must not rename them. + if (options.wslDistro || parseWslPath(worktreePath)) { + return false + } + if (!force) { + try { + // Why: `git worktree remove` re-checks cleanliness as it removes; prove the same thing here or leave removal to Git. + await assertWorktreeCleanForRemoval(worktreePath, false, options) + } catch { + return false + } + } + + const trashPath = await withWorktreeRemoveStageSpan('trash_rename', 'local', () => + moveWorktreeDirectoryToTrash(worktreePath) + ) + if (!trashPath) { + return false + } + try { + await clearGitRegistrationForMissingWorktree(repoPath, worktreePath, options) + } catch (error) { + // Why: put the checkout back so the in-place removal below still sees the worktree Git registered. + if (await restoreWorktreeDirectoryFromTrash(trashPath, worktreePath)) { + return false + } + throw error + } + scheduleWorktreeTrashDeletion(trashPath) + return true +} + +async function clearGitRegistrationForMissingWorktree( + repoPath: string, + worktreePath: string, + options: RemoveWorktreeOptions +): Promise { + const registrationOptions = { + ...options, + timeout: options.timeout ?? WORKTREE_REMOVAL_REGISTRATION_TIMEOUT_MS + } + try { + // Removing an already-missing directory is accepted back to the Git 2.25 baseline and touches only this entry. + await gitExecFileAsync( + ['worktree', 'remove', '--force', worktreePath], + gitExecOptions(repoPath, registrationOptions) + ) + return + } catch (error) { + console.warn( + `[git] Failed to deregister the moved worktree "${worktreePath}"; pruning instead`, + error + ) + } + + await gitExecFileAsync(['worktree', 'prune'], gitExecOptions(repoPath, registrationOptions)) + // Strict (not the shared scan): an unreadable repo must not read as proof that the row is gone. + const stillRegistered = (await listWorktreesStrict(repoPath, registrationOptions)).some( + (worktree) => areWorktreePathsEqual(worktree.path, worktreePath) + ) + if (stillRegistered) { + throw new Error(`Git still reports a registration for "${worktreePath}" after pruning it.`) + } +} + async function deleteBranchAfterWorktreeRemoval( repoPath: string, branchName: string, diff --git a/src/main/index.ts b/src/main/index.ts index 6e769cd39d6..87a6b3fc345 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -221,6 +221,7 @@ import { getOrcaManagedCodexHomePath, getSystemCodexHomePath } from './codex/cod import { normalizeRuntimePathForComparison } from '../shared/cross-platform-path' import type { AgentProviderSessionMetadata } from '../shared/agent-session-resume' import { getDefaultWslDistro } from './wsl' +import { collectWorktreeTrashSweepRoots, sweepStaleWorktreeTrash } from './worktree-trash' import { ClaudeAccountService } from './claude-accounts/service' import { ClaudeRuntimeAuthService } from './claude-accounts/runtime-auth-service' import { @@ -2627,6 +2628,13 @@ void app.whenReady().then(async () => { // Why: externally started serve-sim processes must stay independent — only Orca-managed/attached helpers belong to a workspace. const emulatorBridge = new EmulatorBridge() runtimeService.setEmulatorBridge(emulatorBridge) + // Why: worktree deletion renames the checkout aside and deletes it in the background, so a quit or + // crash mid-delete can leave the moved directory on disk. + void sweepStaleWorktreeTrash( + collectWorktreeTrashSweepRoots(store.getRepos(), store.getSettings()) + ).catch((error) => { + console.warn('[worktrees] Failed to sweep leftover worktree directories:', error) + }) nativeTheme.themeSource = store.getSettings().theme ?? 'system' if (codexRuntimeHome.isHostSystemDefaultRealHomeSelected()) { // Why: establish capability before managed-hook reconciliation so an diff --git a/src/main/observability/instrumentation.ts b/src/main/observability/instrumentation.ts index d4ab5d4e497..369312476f2 100644 --- a/src/main/observability/instrumentation.ts +++ b/src/main/observability/instrumentation.ts @@ -214,6 +214,7 @@ export type WorktreeRemoveStage = | 'git_remove' | 'metadata_purge' | 'pty_sweep' + | 'trash_rename' | 'watcher_gate' /** Wrap one stage of a worktree removal. Children share the parent's `kind` so `kind`-filtered diff --git a/src/main/worktree-trash.test.ts b/src/main/worktree-trash.test.ts new file mode 100644 index 00000000000..49a8af75a1d --- /dev/null +++ b/src/main/worktree-trash.test.ts @@ -0,0 +1,220 @@ +import { existsSync } from 'node:fs' +import { mkdir, mkdtemp, readdir, rm, symlink, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import type { Repo } from '../shared/types' +import { + collectWorktreeTrashSweepRoots, + getWorktreeTrashRoot, + isWorktreeTrashEntryName, + moveWorktreeDirectoryToTrash, + restoreWorktreeDirectoryFromTrash, + scheduleWorktreeTrashDeletion, + sweepStaleWorktreeTrash, + whenWorktreeTrashDeletionsSettled, + WORKTREE_TRASH_DIR_NAME +} from './worktree-trash' + +let scratchDir = '' + +beforeEach(async () => { + scratchDir = await mkdtemp(join(tmpdir(), 'orca-worktree-trash-')) +}) + +afterEach(async () => { + await whenWorktreeTrashDeletionsSettled() + await rm(scratchDir, { recursive: true, force: true }) +}) + +async function createWorktreeDirectory(worktreePath: string): Promise { + await mkdir(join(worktreePath, 'node_modules', 'pkg'), { recursive: true }) + await writeFile(join(worktreePath, 'node_modules', 'pkg', 'index.js'), 'module.exports = 1\n') +} + +describe('moveWorktreeDirectoryToTrash', () => { + it('renames the checkout into a hidden sibling trash root', async () => { + const worktreePath = join(scratchDir, 'repo', 'feature') + await createWorktreeDirectory(worktreePath) + + const trashPath = await moveWorktreeDirectoryToTrash(worktreePath) + + expect(trashPath).toBeDefined() + expect(existsSync(worktreePath)).toBe(false) + expect(trashPath!.startsWith(join(scratchDir, 'repo', WORKTREE_TRASH_DIR_NAME))).toBe(true) + expect(existsSync(join(trashPath!, 'node_modules', 'pkg', 'index.js'))).toBe(true) + }) + + it('generates sweepable, collision-free entry names', async () => { + const first = await moveWorktreeDirectoryToTrash(await seededWorktree('one')) + const second = await moveWorktreeDirectoryToTrash(await seededWorktree('two')) + + expect(first).not.toEqual(second) + expect(await readdir(join(scratchDir, 'repo', WORKTREE_TRASH_DIR_NAME))).toHaveLength(2) + for (const entry of await readdir(join(scratchDir, 'repo', WORKTREE_TRASH_DIR_NAME))) { + expect(isWorktreeTrashEntryName(entry)).toBe(true) + } + }) + + it('reports the rename as unavailable and leaves no trash root when the checkout is missing', async () => { + const worktreePath = join(scratchDir, 'repo', 'gone') + + expect(await moveWorktreeDirectoryToTrash(worktreePath)).toBeUndefined() + expect(existsSync(getWorktreeTrashRoot(worktreePath))).toBe(false) + }) + + it.skipIf(process.platform === 'win32')('refuses a symlinked trash root', async () => { + const worktreePath = join(scratchDir, 'repo', 'feature') + const externalRoot = join(scratchDir, 'external') + await createWorktreeDirectory(worktreePath) + await mkdir(externalRoot) + await symlink(externalRoot, getWorktreeTrashRoot(worktreePath)) + + expect(await moveWorktreeDirectoryToTrash(worktreePath)).toBeUndefined() + expect(existsSync(worktreePath)).toBe(true) + expect(await readdir(externalRoot)).toEqual([]) + }) + + async function seededWorktree(name: string): Promise { + const worktreePath = join(scratchDir, 'repo', name) + await createWorktreeDirectory(worktreePath) + return worktreePath + } +}) + +describe('restoreWorktreeDirectoryFromTrash', () => { + it('puts the checkout back where Git registered it', async () => { + const worktreePath = join(scratchDir, 'repo', 'feature') + await createWorktreeDirectory(worktreePath) + const trashPath = await moveWorktreeDirectoryToTrash(worktreePath) + + expect(await restoreWorktreeDirectoryFromTrash(trashPath!, worktreePath)).toBe(true) + expect(existsSync(join(worktreePath, 'node_modules', 'pkg', 'index.js'))).toBe(true) + expect(existsSync(trashPath!)).toBe(false) + }) + + it('reports failure instead of throwing when the trashed path is gone', async () => { + const trashRoot = getWorktreeTrashRoot(join(scratchDir, 'repo', 'feature')) + expect( + await restoreWorktreeDirectoryFromTrash( + join(trashRoot, 'wt-1-abcdef01'), + join(scratchDir, 'repo', 'feature') + ) + ).toBe(false) + }) +}) + +describe('scheduleWorktreeTrashDeletion', () => { + it('deletes trashed checkouts in the background', async () => { + const worktreePath = join(scratchDir, 'repo', 'feature') + await createWorktreeDirectory(worktreePath) + const trashPath = await moveWorktreeDirectoryToTrash(worktreePath) + + scheduleWorktreeTrashDeletion(trashPath!) + await whenWorktreeTrashDeletionsSettled() + + expect(existsSync(trashPath!)).toBe(false) + expect(existsSync(getWorktreeTrashRoot(worktreePath))).toBe(true) + }) +}) + +describe('sweepStaleWorktreeTrash', () => { + it('removes leftover entries from both workspace layouts and nothing else', async () => { + const nestedTrashRoot = join(scratchDir, 'repo', WORKTREE_TRASH_DIR_NAME) + const flatTrashRoot = join(scratchDir, WORKTREE_TRASH_DIR_NAME) + await mkdir(join(nestedTrashRoot, 'wt-1700000000000-abcdef01', 'src'), { recursive: true }) + await mkdir(join(flatTrashRoot, 'wt-1700000000001-abcdef02'), { recursive: true }) + // Not generated by Orca: a stray directory and file inside the trash root must survive. + await mkdir(join(nestedTrashRoot, 'unrelated-directory'), { recursive: true }) + await writeFile(join(nestedTrashRoot, 'wt-notes.txt'), 'keep me\n') + const liveWorktree = join(scratchDir, 'repo', 'feature') + await createWorktreeDirectory(liveWorktree) + + const { removed } = await sweepStaleWorktreeTrash([scratchDir]) + + expect(removed).toBe(2) + expect(existsSync(join(nestedTrashRoot, 'wt-1700000000000-abcdef01'))).toBe(false) + expect(existsSync(join(flatTrashRoot, 'wt-1700000000001-abcdef02'))).toBe(false) + expect(existsSync(join(nestedTrashRoot, 'unrelated-directory'))).toBe(true) + expect(existsSync(join(nestedTrashRoot, 'wt-notes.txt'))).toBe(true) + expect(existsSync(join(liveWorktree, 'node_modules', 'pkg', 'index.js'))).toBe(true) + }) + + it('ignores workspace roots that do not exist', async () => { + expect(await sweepStaleWorktreeTrash([join(scratchDir, 'missing')])).toEqual({ removed: 0 }) + }) + + it('never descends past the trash roots beside worktrees', async () => { + const deepTrashRoot = join(scratchDir, 'repo', 'feature', WORKTREE_TRASH_DIR_NAME) + await mkdir(join(deepTrashRoot, 'wt-1700000000002-abcdef03'), { recursive: true }) + + expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 0 }) + expect(existsSync(join(deepTrashRoot, 'wt-1700000000002-abcdef03'))).toBe(true) + }) + + it.skipIf(process.platform === 'win32')( + 'does not sweep through a symlinked trash root', + async () => { + const externalEntry = join(scratchDir, 'external', 'wt-1700000000003-abcdef04') + await mkdir(externalEntry, { recursive: true }) + await symlink(join(scratchDir, 'external'), join(scratchDir, WORKTREE_TRASH_DIR_NAME)) + + expect(await sweepStaleWorktreeTrash([scratchDir])).toEqual({ removed: 0 }) + expect(existsSync(externalEntry)).toBe(true) + } + ) +}) + +describe('collectWorktreeTrashSweepRoots', () => { + const settings = { workspaceDir: '/home/dev/orca/workspaces', nestWorkspaces: true } + + function repo(overrides: Partial): Repo { + return { id: 'repo-1', path: '/home/dev/code/orca', ...overrides } as unknown as Repo + } + + it('collects one root per local git repo', () => { + expect( + collectWorktreeTrashSweepRoots( + [repo({}), repo({ id: 'repo-2', path: '/code/other' })], + settings + ) + ).toEqual(['/home/dev/orca/workspaces']) + }) + + it('honours a repo-specific worktree base path', () => { + expect( + collectWorktreeTrashSweepRoots([repo({ worktreeBasePath: '/volumes/fast/trees' })], settings) + ).toEqual(['/volumes/fast/trees']) + }) + + it('skips SSH repos and folder workspaces', () => { + expect( + collectWorktreeTrashSweepRoots( + [repo({ connectionId: 'ssh-1' }), repo({ id: 'repo-3', kind: 'folder' })], + settings + ) + ).toEqual([]) + }) + + it('skips WSL repos that cannot create host trash', () => { + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + try { + expect( + collectWorktreeTrashSweepRoots( + [ + repo({ path: '\\\\wsl.localhost\\Ubuntu\\home\\dev\\orca' }), + repo({ + id: 'repo-2', + path: 'C:\\code\\orca', + worktreeBasePath: '\\\\wsl.localhost\\Ubuntu\\home\\dev\\trees' + }) + ], + settings + ) + ).toEqual([]) + } finally { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + } + }) +}) diff --git a/src/main/worktree-trash.ts b/src/main/worktree-trash.ts new file mode 100644 index 00000000000..5c0aa32be2b --- /dev/null +++ b/src/main/worktree-trash.ts @@ -0,0 +1,175 @@ +// Why: `git worktree remove` deletes the whole checkout (usually a multi-GB node_modules) inside the +// remove IPC, so the UI sat on a spinner for 8-35s. Renaming the directory into a sibling trash root +// is a metadata operation, and the recursive delete then runs after the IPC has already returned. + +import { randomBytes } from 'node:crypto' +import { lstat, mkdir, readdir, rename, rmdir } from 'node:fs/promises' +import { dirname, join } from 'node:path' +import { removeHostTree } from './host-tree-removal' +import { isFolderRepo } from '../shared/repo-kind' +import { computeWorkspaceRoot, getWorktreePathSettings } from './ipc/worktree-logic' +import type { GlobalSettings, Repo } from '../shared/types' +import { parseWslPath } from './wsl' + +export const WORKTREE_TRASH_DIR_NAME = '.orca-worktree-trash' + +// `-`: the nonce keeps concurrent removals of same-named worktrees from colliding. +const TRASH_ENTRY_PATTERN = /^wt-\d+-[0-9a-f]{8}$/ + +// Why: the sweep must stay cheap on a workspace root holding many repo containers. +const TRASH_SWEEP_MAX_CONTAINERS = 200 + +/** Trash root for a worktree: a hidden sibling, so the rename always stays on one volume. */ +export function getWorktreeTrashRoot(worktreePath: string): string { + return join(dirname(worktreePath), WORKTREE_TRASH_DIR_NAME) +} + +export function isWorktreeTrashEntryName(entryName: string): boolean { + return TRASH_ENTRY_PATTERN.test(entryName) +} + +/** + * Move a worktree directory aside so the caller can return before it is deleted. + * Returns the trash path, or `undefined` when the rename is unavailable (a different + * volume, or Windows open handles) and the caller must delete in place instead. + */ +export async function moveWorktreeDirectoryToTrash( + worktreePath: string +): Promise { + const trashRoot = getWorktreeTrashRoot(worktreePath) + const trashPath = join(trashRoot, `wt-${Date.now()}-${randomBytes(4).toString('hex')}`) + try { + await mkdir(trashRoot, { recursive: true }) + const trashRootStat = await lstat(trashRoot) + if (!trashRootStat.isDirectory() || trashRootStat.isSymbolicLink()) { + throw new Error(`Refusing non-directory worktree trash root: ${trashRoot}`) + } + await rename(worktreePath, trashPath) + return trashPath + } catch (error) { + console.warn( + `[worktrees] Deferred deletion unavailable for ${worktreePath}; deleting in place`, + error + ) + // Leave no empty trash root behind when the rename never happened; rmdir keeps queued entries. + await rmdir(trashRoot).catch(() => {}) + return undefined + } +} + +/** Undo a trash rename so a failed registration cleanup leaves the worktree exactly as it was. */ +export async function restoreWorktreeDirectoryFromTrash( + trashPath: string, + worktreePath: string +): Promise { + try { + await rename(trashPath, worktreePath) + return true + } catch (error) { + console.warn(`[worktrees] Failed to restore ${worktreePath} from ${trashPath}`, error) + return false + } +} + +// Why serialized: one background delete at a time keeps a burst of removals from saturating disk I/O +// while the user keeps working. +let queuedTrashDeletions: Promise = Promise.resolve() + +export function scheduleWorktreeTrashDeletion(trashPath: string): void { + queuedTrashDeletions = queuedTrashDeletions.then(async () => { + try { + await removeHostTree(trashPath) + } catch (error) { + // Why only a warning: the directory is already invisible to the user, and the startup sweep retries it. + console.warn(`[worktrees] Failed to delete trashed worktree at ${trashPath}`, error) + } + }) +} + +/** Test/shutdown hook: resolves once every queued background deletion has settled. */ +export function whenWorktreeTrashDeletionsSettled(): Promise { + return queuedTrashDeletions +} + +/** + * Delete trash entries left behind by a previous run (a crash or a kill during background deletion). + * Only entries matching the generated name pattern inside a trash root are removed. + */ +export async function sweepStaleWorktreeTrash( + workspaceRoots: readonly string[] +): Promise<{ removed: number }> { + let removed = 0 + for (const trashRoot of await collectExistingTrashRoots(workspaceRoots)) { + let entries: string[] + try { + const trashRootStat = await lstat(trashRoot) + if (!trashRootStat.isDirectory() || trashRootStat.isSymbolicLink()) { + continue + } + entries = await readdir(trashRoot) + } catch { + continue + } + for (const entry of entries) { + if (!isWorktreeTrashEntryName(entry)) { + continue + } + try { + await removeHostTree(join(trashRoot, entry)) + removed += 1 + } catch (error) { + console.warn( + `[worktrees] Failed to sweep leftover worktree at ${trashRoot}/${entry}`, + error + ) + } + } + } + if (removed > 0) { + console.log(`[worktrees] Swept ${removed} leftover worktree director(ies) from a previous run`) + } + return { removed } +} + +/** Trash roots live beside worktrees, so they sit at the workspace root (flat) or one level in (nested). */ +async function collectExistingTrashRoots(workspaceRoots: readonly string[]): Promise { + const trashRoots = new Set() + for (const workspaceRoot of new Set(workspaceRoots)) { + trashRoots.add(join(workspaceRoot, WORKTREE_TRASH_DIR_NAME)) + let containers: string[] = [] + try { + containers = (await readdir(workspaceRoot, { withFileTypes: true })) + .filter((entry) => entry.isDirectory() && entry.name !== WORKTREE_TRASH_DIR_NAME) + .slice(0, TRASH_SWEEP_MAX_CONTAINERS) + .map((entry) => entry.name) + } catch { + continue + } + for (const container of containers) { + trashRoots.add(join(workspaceRoot, container, WORKTREE_TRASH_DIR_NAME)) + } + } + return [...trashRoots] +} + +/** Workspace roots of local git repos — the only places Orca creates worktree trash. */ +export function collectWorktreeTrashSweepRoots( + repos: readonly Repo[], + settings: Pick +): string[] { + const roots = new Set() + for (const repo of repos) { + if (repo.connectionId || isFolderRepo(repo) || parseWslPath(repo.path)) { + continue + } + try { + const workspaceRoot = computeWorkspaceRoot(repo.path, getWorktreePathSettings(repo, settings)) + if (!parseWslPath(workspaceRoot)) { + roots.add(workspaceRoot) + } + } catch { + // A repo with an unusable configured base path simply has no trash root to sweep. + } + } + return [...roots] +} diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index 906a9b78811..660a2b7aaad 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -1,5 +1,5 @@ import { execFile } from 'node:child_process' -import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { mkdtemp, rename, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { promisify } from 'node:util' @@ -137,6 +137,19 @@ describeBinaryCompatibility('real Git binary compatibility', () => { ).resolves.toBeDefined() }) + it('deregisters a worktree whose directory was renamed away', async () => { + // Orca renames the checkout into a trash directory and then clears the registration, so every + // supported Git must accept `worktree remove --force` on the now-missing path. + await runGit(['worktree', 'add', '-b', 'compat-deferred', 'deferred-wt']) + await rename(join(repoPath, 'deferred-wt'), join(repoPath, 'deferred-trash')) + + await expect(runGit(['worktree', 'remove', '--force', 'deferred-wt'])).resolves.toBeDefined() + + const remaining = await runGit(['worktree', 'list', '--porcelain']) + expect(remaining.stdout).not.toContain('deferred-wt') + await rm(join(repoPath, 'deferred-trash'), { recursive: true, force: true }) + }) + it('recognizes ref and merge-tree compatibility boundaries', async () => { await expectPreferredOrRecognizedFallback( ['for-each-ref', '--format=%(refname)', '--exclude=refs/remotes/**/HEAD', '--count=10'],