diff --git a/src/main/ipc/worktrees-failed-removal.test.ts b/src/main/ipc/worktrees-failed-removal.test.ts index a52abc37eb6..c1536fae308 100644 --- a/src/main/ipc/worktrees-failed-removal.test.ts +++ b/src/main/ipc/worktrees-failed-removal.test.ts @@ -15,6 +15,8 @@ import { worktreeCheckoutExists } from '../worktree-removal-table' import type * as WorktreeRemovalModule from '../git/worktree-removal' import type * as WorktreeRemovalTable from '../worktree-removal-table' import type * as WorktreeRemovalLeftover from '../worktree-removal-leftover' +import { removeLocalWorktreePath } from '../local-worktree-filesystem' +import type * as LocalWorktreeFilesystem from '../local-worktree-filesystem' import { _resetPendingWorktreeRemovalsForTests, _settlePendingWorktreeRemovalsForTests, @@ -108,6 +110,10 @@ vi.mock('../worktree-removal-table', async (importOriginal) => ({ ...(await importOriginal()), worktreeCheckoutExists: vi.fn(async () => true) })) +vi.mock('../local-worktree-filesystem', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, removeLocalWorktreePath: vi.fn(actual.removeLocalWorktreePath) } +}) vi.mock('../worktree-removal-leftover', async (importOriginal) => ({ ...(await importOriginal()), isUnregisteredRemovalLeftover: vi.fn(async () => true) @@ -115,6 +121,8 @@ vi.mock('../worktree-removal-leftover', async (importOriginal) => ({ const featureId = 'repo-1::/workspace/feature-wt' const GIT_ERROR = "error: failed to delete '/workspace/feature-wt': Operation not permitted" +const FOLLOW_UP_DELETE_ERROR = + "EPERM: operation not permitted, unlink '/workspace/feature-wt/locked'" function remove(args: Record): Promise { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: worktrees:remove resolves a RemoveWorktreeResult. @@ -182,15 +190,17 @@ describe('a failed delete Git no longer registers, over desktop IPC', () => { listWorktreesMock.mockResolvedValue([main]) throw new Error(GIT_ERROR) }) + // Orca's follow-up delete of the folder Git left also fails, so the row stays listed. + vi.mocked(removeLocalWorktreePath).mockRejectedValueOnce(new Error(FOLLOW_UP_DELETE_ERROR)) - await expect(remove({ worktreeId: featureId })).rejects.toThrow(/Operation not permitted/) + await expect(remove({ worktreeId: featureId })).rejects.toThrow(FOLLOW_UP_DELETE_ERROR) await _settlePendingWorktreeRemovalsForTests() expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', 'remove', pushTarget.remoteName], { cwd: '/workspace/repo' }) expect(await listFeature()).toMatchObject({ - removalError: expect.stringMatching(/Operation not permitted/) + removalError: expect.stringContaining(FOLLOW_UP_DELETE_ERROR) }) }) diff --git a/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts b/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts index 7cd7941d0b7..5581eab1a0e 100644 --- a/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts +++ b/src/main/ipc/worktrees/removal/remove-registered-local-worktree.ts @@ -16,7 +16,7 @@ import { } from '../../../git/worktree' import { getWorktreeSharedLinkPaths } from '../../../git/worktree-shared-directories' import { cleanupLocalOrphanedWorktreeDirectory } from '../../../local-orphaned-worktree-cleanup' -import { recoverLocalWindowsWorktreeRemoval } from '../../../local-worktree-removal-recovery' +import { recoverPartialLocalWorktreeRemoval } from '../../../local-worktree-removal-recovery' import { withWorktreeRemoveStageSpan } from '../../../observability/instrumentation' import { assertNestedWorktreeRemovalApproval } from '../../../nested-worktree-removal-plan' import { findRegisteredDeletableWorktree } from '../../../worktree-removal-safety' @@ -233,8 +233,8 @@ async function finishLocalWorktreeRemoval({ if (usesNativeWindowsRemoval(canonicalWorktreePath, localWorktreeGitOptions)) { throw error } - // Git for Windows may deregister before recursive filesystem deletion fails. - const recoveredRemovalResult = await recoverLocalWindowsWorktreeRemoval({ + // Git deregisters even when its recursive filesystem deletion fails partway. + const recoveredRemovalResult = await recoverPartialLocalWorktreeRemoval({ error, force: args.force ?? false, canonicalWorktreePath, diff --git a/src/main/local-worktree-removal-recovery-live.test.ts b/src/main/local-worktree-removal-recovery-live.test.ts index 54ab64f9719..96f404c403e 100644 --- a/src/main/local-worktree-removal-recovery-live.test.ts +++ b/src/main/local-worktree-removal-recovery-live.test.ts @@ -6,7 +6,7 @@ import { tmpdir } from 'node:os' import path from 'node:path' import { setTimeout as delay } from 'node:timers/promises' import { afterEach, describe, expect, it } from 'vitest' -import { recoverLocalWindowsWorktreeRemoval } from './local-worktree-removal-recovery' +import { recoverPartialLocalWorktreeRemoval } from './local-worktree-removal-recovery' const tempRoots: string[] = [] @@ -105,7 +105,7 @@ describe('local Windows worktree removal recovery (live Git)', () => { await stopProcess(churn) await expect( - recoverLocalWindowsWorktreeRemoval({ + recoverPartialLocalWorktreeRemoval({ error: Object.assign(new Error('git worktree remove failed'), { stderr: removal.stderr }), diff --git a/src/main/local-worktree-removal-recovery.test.ts b/src/main/local-worktree-removal-recovery.test.ts index 0ec83a62af8..b63c8cce37c 100644 --- a/src/main/local-worktree-removal-recovery.test.ts +++ b/src/main/local-worktree-removal-recovery.test.ts @@ -24,9 +24,10 @@ vi.mock('./git/worktree', () => ({ })) import { - recoverLocalWindowsWorktreeRemoval, + recoverPartialLocalWorktreeRemoval, removeStaleLocalWorktreeRegistration } from './local-worktree-removal-recovery' +import { NESTED_WORKTREE_REMOVAL_PREFIX } from '../shared/worktree/nested-removal' async function withPlatform(platform: NodeJS.Platform, fn: () => Promise): Promise { const original = process.platform @@ -38,7 +39,7 @@ async function withPlatform(platform: NodeJS.Platform, fn: () => Promise): } } -describe('recoverLocalWindowsWorktreeRemoval', () => { +describe('recoverPartialLocalWorktreeRemoval', () => { beforeEach(() => { gitExecFileAsyncMock.mockReset() listWorktreesStrictMock.mockReset() @@ -58,7 +59,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { stderr: "error: failed to delete 'C:/repo/worktree/delete-e2e-held-cwd': Permission denied" }) - const result = await recoverLocalWindowsWorktreeRemoval({ + const result = await recoverPartialLocalWorktreeRemoval({ error, force: true, canonicalWorktreePath: 'C:/repo/worktree/delete-e2e-held-cwd', @@ -74,15 +75,15 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { {}, undefined ) - expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], { - cwd: 'C:/repo' - }) - expect(result).toEqual({ - preservedBranch: { - branchName: 'delete-e2e-held-cwd', - head: 'abc123' - } - }) + expect(gitExecFileAsyncMock).not.toHaveBeenCalledWith( + ['worktree', 'prune'], + expect.anything() + ) + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['branch', '-d', '--', 'delete-e2e-held-cwd'], + expect.objectContaining({ cwd: 'C:/repo' }) + ) + expect(result).toEqual({}) }) }) @@ -93,7 +94,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { "error: failed to delete 'C:/repo/worktree/delete-e2e-held-cwd': Directory not empty" }) - const result = await recoverLocalWindowsWorktreeRemoval({ + const result = await recoverPartialLocalWorktreeRemoval({ error, force: false, canonicalWorktreePath: 'C:/repo/worktree/delete-e2e-held-cwd', @@ -109,15 +110,15 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { {}, undefined ) - expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], { - cwd: 'C:/repo' - }) - expect(result).toEqual({ - preservedBranch: { - branchName: 'delete-e2e-held-cwd', - head: 'abc123' - } - }) + expect(gitExecFileAsyncMock).not.toHaveBeenCalledWith( + ['worktree', 'prune'], + expect.anything() + ) + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['branch', '-d', '--', 'delete-e2e-held-cwd'], + expect.objectContaining({ cwd: 'C:/repo' }) + ) + expect(result).toEqual({}) }) }) @@ -134,7 +135,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { }) await withPlatform('win32', async () => { await expect( - recoverLocalWindowsWorktreeRemoval({ + recoverPartialLocalWorktreeRemoval({ error: new Error('Git deregistered but left the checkout'), force: true, canonicalWorktreePath: 'C:/repo/worktree/feature', @@ -162,7 +163,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { const watcherError = new Error('file watcher process did not exit after termination deadline') await expect( - recoverLocalWindowsWorktreeRemoval({ + recoverPartialLocalWorktreeRemoval({ error: new Error('git worktree remove failed'), force: true, canonicalWorktreePath: 'C:/repo/worktree/feature', @@ -181,7 +182,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { it('recovers localized Windows failures after Git already removed the registration', async () => { await withPlatform('win32', async () => { - const result = await recoverLocalWindowsWorktreeRemoval({ + const result = await recoverPartialLocalWorktreeRemoval({ error: Object.assign(new Error('git worktree remove failed'), { stderr: "Fehler: 'C:/repo/worktree/feature' konnte nicht gelöscht werden" }), @@ -216,7 +217,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { ]) await expect( - recoverLocalWindowsWorktreeRemoval({ + recoverPartialLocalWorktreeRemoval({ error: Object.assign(new Error('git worktree remove failed'), { stderr: 'Fehler: unerwarteter Git-Zustand' }), @@ -233,24 +234,90 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { }) }) - it('does not recover partial filesystem deletion wording off Windows', async () => { - await withPlatform('linux', async () => { - const error = Object.assign(new Error('git worktree remove failed'), { - stderr: "error: failed to delete 'C:/repo/worktree/delete-e2e-held-cwd': Permission denied" + it('finishes a macOS deletion Git deregistered before failing with Directory not empty', async () => { + await withPlatform('darwin', async () => { + const closeWatcher = vi.fn().mockResolvedValue(undefined) + + const result = await recoverPartialLocalWorktreeRemoval({ + error: Object.assign(new Error('git worktree remove failed'), { + stderr: + "error: failed to delete '/Users/dev/orca/workspaces/feature': Directory not empty" + }), + force: true, + canonicalWorktreePath: '/Users/dev/orca/workspaces/feature', + repoPath: '/Users/dev/repo', + localWorktreeGitOptions: {}, + registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' }, + deleteBranch: true, + closeWatcher }) + expect(closeWatcher).toHaveBeenCalledWith('/Users/dev/orca/workspaces/feature') + expect(removeLocalWorktreePathMock).toHaveBeenCalledWith( + '/Users/dev/orca/workspaces/feature', + {}, + undefined + ) + expect(gitExecFileAsyncMock).toHaveBeenCalledWith( + ['branch', '-d', '--', 'feature'], + expect.objectContaining({ cwd: '/Users/dev/repo' }) + ) + expect(result).toEqual({}) + }) + }) + + it('leaves an orphaned checkout to the orphan cleanup instead of deleting it', async () => { + await withPlatform('darwin', async () => { + const closeWatcher = vi.fn().mockResolvedValue(undefined) + await expect( - recoverLocalWindowsWorktreeRemoval({ - error, + recoverPartialLocalWorktreeRemoval({ + error: Object.assign(new Error('git worktree remove failed'), { + stderr: "fatal: '/Users/dev/orca/workspaces/feature' is not a working tree" + }), force: true, - canonicalWorktreePath: 'C:/repo/worktree/delete-e2e-held-cwd', - repoPath: 'C:/repo', + canonicalWorktreePath: '/Users/dev/orca/workspaces/feature', + repoPath: '/Users/dev/repo', localWorktreeGitOptions: {}, - registeredWorktree: { branch: 'refs/heads/delete-e2e-held-cwd', head: 'abc123' }, + registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' }, deleteBranch: true, - closeWatcher: vi.fn().mockResolvedValue(undefined) + closeWatcher }) ).resolves.toBeUndefined() + expect(closeWatcher).not.toHaveBeenCalled() + expect(removeLocalWorktreePathMock).not.toHaveBeenCalled() + }) + }) + + it('does not delete a checkout that now contains a registered worktree', async () => { + await withPlatform('darwin', async () => { + listWorktreesStrictMock.mockResolvedValue([ + { + path: '/Users/dev/orca/workspaces/feature/nested', + head: 'def456', + branch: 'refs/heads/nested', + isBare: false, + isMainWorktree: false + } + ]) + const closeWatcher = vi.fn().mockResolvedValue(undefined) + + await expect( + recoverPartialLocalWorktreeRemoval({ + error: Object.assign(new Error('git worktree remove failed'), { + stderr: + "error: failed to delete '/Users/dev/orca/workspaces/feature': Directory not empty" + }), + force: true, + canonicalWorktreePath: '/Users/dev/orca/workspaces/feature', + repoPath: '/Users/dev/repo', + localWorktreeGitOptions: {}, + registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' }, + deleteBranch: true, + closeWatcher + }) + ).rejects.toThrow(NESTED_WORKTREE_REMOVAL_PREFIX) + expect(closeWatcher).not.toHaveBeenCalled() expect(removeLocalWorktreePathMock).not.toHaveBeenCalled() }) }) @@ -266,7 +333,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { const closeWatcher = vi.fn().mockResolvedValue(undefined) await expect( - recoverLocalWindowsWorktreeRemoval({ + recoverPartialLocalWorktreeRemoval({ error: Object.assign(new Error('git worktree remove failed'), { stderr: 'error: failed to delete deep/file.txt: Filename too long' }), @@ -306,7 +373,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { const closeWatcher = vi.fn().mockResolvedValue(undefined) await expect( - recoverLocalWindowsWorktreeRemoval({ + recoverPartialLocalWorktreeRemoval({ error: Object.assign(new Error('git worktree remove failed'), { stderr }), force: true, canonicalWorktreePath: 'C:/repo/worktree/feature', @@ -331,7 +398,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { const closeWatcher = vi.fn().mockResolvedValue(undefined) await expect( - recoverLocalWindowsWorktreeRemoval({ + recoverPartialLocalWorktreeRemoval({ error: Object.assign(new Error('git worktree remove failed'), { stderr: "error: failed to delete 'C:/repo/worktree/feature': Directory not empty" }), diff --git a/src/main/local-worktree-removal-recovery.ts b/src/main/local-worktree-removal-recovery.ts index 4fc6de09b39..14e957c79a0 100644 --- a/src/main/local-worktree-removal-recovery.ts +++ b/src/main/local-worktree-removal-recovery.ts @@ -3,11 +3,14 @@ import { pruneGitWorktrees } from './git/worktree-prune-guard' import type { GitWorktreeInfo } from '../shared/worktree/types' import { assertWorktreeUnlockedForRemoval } from '../shared/worktree/removal' import { areWorktreePathsEqual } from './git/worktree-path-comparison' -import { formatWorktreeRemovalError } from './ipc/worktree-logic' +import { formatWorktreeRemovalError, isOrphanedWorktreeError } from './ipc/worktree-logic' import { listWorktreesStrict, type GitWorktreeExecOptions } from './git/worktree' +import { normalizeLocalBranchRef } from './git/worktree-operation-options' +import { deleteBranchOfUnregisteredWorktree } from './git/worktree-removal' import { removeLocalWorktreePath } from './local-worktree-filesystem' +import { assertWorktreeDoesNotContainRegisteredWorktree } from './worktree-removal-safety' -type LocalWindowsRemovalRecoveryArgs = { +type LocalPartialRemovalRecoveryArgs = { error: unknown force: boolean canonicalWorktreePath: string @@ -20,7 +23,7 @@ type LocalWindowsRemovalRecoveryArgs = { } type StaleLocalWorktreeRegistrationArgs = Omit< - LocalWindowsRemovalRecoveryArgs, + LocalPartialRemovalRecoveryArgs, 'error' | 'force' | 'closeWatcher' | 'signal' > @@ -39,16 +42,12 @@ function preservedBranchResult( } } -function staleRegistrationRecoveryError( - error: unknown, - canonicalWorktreePath: string, - force: boolean -): Error { +function staleRegistrationRecoveryError(error: unknown, canonicalWorktreePath: string): Error { return new Error( `${formatWorktreeRemovalError( error, canonicalWorktreePath, - force + true )} Git still has stale worktree registration. Retry deletion after resolving the Git registration error.` ) } @@ -56,8 +55,7 @@ function staleRegistrationRecoveryError( async function verifyGitWorktreeRegistrationRemoved( repoPath: string, localWorktreeGitOptions: GitWorktreeExecOptions, - canonicalWorktreePath: string, - force: boolean + canonicalWorktreePath: string ): Promise { try { const remainingWorktrees = await listWorktreesStrict(repoPath, localWorktreeGitOptions) @@ -69,13 +67,12 @@ async function verifyGitWorktreeRegistrationRemoved( throw new Error('Git still reports the worktree registration after cleanup.') } } catch (error) { - throw staleRegistrationRecoveryError(error, canonicalWorktreePath, force) + throw staleRegistrationRecoveryError(error, canonicalWorktreePath) } } -async function removeRequiredGitWorktreeRegistration( - args: StaleLocalWorktreeRegistrationArgs, - forceForError = true +export async function removeStaleLocalWorktreeRegistration( + args: StaleLocalWorktreeRegistrationArgs ): Promise { assertWorktreeUnlockedForRemoval(args.registeredWorktree) @@ -93,25 +90,24 @@ async function removeRequiredGitWorktreeRegistration( await verifyGitWorktreeRegistrationRemoved( args.repoPath, args.localWorktreeGitOptions, - args.canonicalWorktreePath, - forceForError + args.canonicalWorktreePath ) } catch (verificationError) { throw removalError - ? staleRegistrationRecoveryError(removalError, args.canonicalWorktreePath, forceForError) + ? staleRegistrationRecoveryError(removalError, args.canonicalWorktreePath) : verificationError } // Preserve the branch when Git may have detached the row before reporting a filesystem error. return result ?? preservedBranchResult(args.registeredWorktree, args.deleteBranch) } -export async function recoverLocalWindowsWorktreeRemoval( - args: LocalWindowsRemovalRecoveryArgs +export async function recoverPartialLocalWorktreeRemoval( + args: LocalPartialRemovalRecoveryArgs ): Promise { // Reject Git locks before recovery can recursively delete files. assertWorktreeUnlockedForRemoval(args.registeredWorktree) - if (!(await isRecoverableWindowsFilesystemRemovalFailure(args))) { + if (!(await isRecoverablePartialRemovalFailure(args))) { return undefined } @@ -126,29 +122,41 @@ export async function recoverLocalWindowsWorktreeRemoval( } catch (error) { throw new Error(formatWorktreeRemovalError(error, args.canonicalWorktreePath, args.force)) } - return removeRequiredGitWorktreeRegistration(args, args.force) + const { branch, head } = args.registeredWorktree + // Same -d policy as a normal delete: merged branches go, unmerged ones are preserved. + return deleteBranchOfUnregisteredWorktree( + args.repoPath, + args.canonicalWorktreePath, + args.deleteBranch && branch ? { name: normalizeLocalBranchRef(branch), head } : null, + args.localWorktreeGitOptions + ) } -async function isRecoverableWindowsFilesystemRemovalFailure( - args: LocalWindowsRemovalRecoveryArgs +async function isRecoverablePartialRemovalFailure( + args: LocalPartialRemovalRecoveryArgs ): Promise { - if (process.platform !== 'win32' || typeof args.error !== 'object' || args.error === null) { + // Leave English orphan errors (Git refused before deleting) to the proven-orphan cleanup. + if ( + typeof args.error !== 'object' || + args.error === null || + isOrphanedWorktreeError(args.error) + ) { return false } + let worktrees: GitWorktreeInfo[] try { - const worktrees = await listWorktreesStrict(args.repoPath, args.localWorktreeGitOptions) - // Localized error text cannot prove removal started; require the Git row to be absent. - return !worktrees.some((worktree) => - areWorktreePathsEqual(worktree.path, args.canonicalWorktreePath) - ) + worktrees = await listWorktreesStrict(args.repoPath, args.localWorktreeGitOptions) } catch { return false } -} - -export async function removeStaleLocalWorktreeRegistration( - args: StaleLocalWorktreeRegistrationArgs -): Promise { - return removeRequiredGitWorktreeRegistration(args) + // Localized error text cannot prove removal started; require the Git row to be absent. + if ( + worktrees.some((worktree) => areWorktreePathsEqual(worktree.path, args.canonicalWorktreePath)) + ) { + return false + } + // A worktree registered inside the checkout during Git's delete must not be removed with it. + assertWorktreeDoesNotContainRegisteredWorktree(args.canonicalWorktreePath, worktrees) + return true } diff --git a/src/main/runtime/orca-runtime-tests/worktree-removal-failed-retry.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-removal-failed-retry.spec.ts index ffd2599e84a..4d00bcb9c68 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-removal-failed-retry.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-removal-failed-retry.spec.ts @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { join, listWorktreesStrict, + localWorktreeFilesystem, mkdir, mkdtemp, removeWorktree, @@ -26,6 +27,7 @@ import { } from '../../worktree-removal-records' const FAILURE = "error: failed to delete 'node_modules/a/LICENSE': Operation not permitted" +const FOLLOW_UP_DELETE_FAILURE = "EPERM: operation not permitted, unlink 'node_modules/a/LICENSE'" describe('runtime Delete on a failed delete’s leftover', () => { let directory = '' @@ -203,18 +205,22 @@ describe('runtime listing straight after a delete fails partway', () => { gitLists([]) throw new Error(FAILURE) }) + // Orca's follow-up delete of the folder Git left also fails, so the row stays listed. + vi.spyOn(localWorktreeFilesystem, 'removeLocalWorktreePath').mockRejectedValueOnce( + new Error(FOLLOW_UP_DELETE_FAILURE) + ) await expect( runtime.removeManagedWorktree(`id:${leftoverId}`, { force: true, waitForBackgroundRemoval: true }) - ).rejects.toThrow(/Operation not permitted/) + ).rejects.toThrow(FOLLOW_UP_DELETE_FAILURE) await _settlePendingWorktreeRemovalsForTests() expect(await listLeftover()).toMatchObject({ path: leftover, - removalError: expect.stringMatching(/Operation not permitted/) + removalError: expect.stringContaining(FOLLOW_UP_DELETE_FAILURE) }) }) }) diff --git a/src/main/runtime/runtime-failed-local-worktree-removal.test.ts b/src/main/runtime/runtime-failed-local-worktree-removal.test.ts index c5fe89560ab..a399ab11b8d 100644 --- a/src/main/runtime/runtime-failed-local-worktree-removal.test.ts +++ b/src/main/runtime/runtime-failed-local-worktree-removal.test.ts @@ -54,6 +54,8 @@ let worktreePath = '' let lockedFile = '' let worktreeId = '' let repo: Repo +// Only Orca's follow-up delete names the locked file; Git's error names the checkout. +const LOCKED_FILE_ERROR = /LICENSE could not be deleted/ async function git(args: string[], cwd = repoPath): Promise { const { stdout } = await execFileAsync('git', args, { cwd }) @@ -231,16 +233,19 @@ describe.skipIf(process.platform !== 'darwin')('a worktree delete Git fails part await removeTree(scratchDir) }) - it('keeps the leftover listed with Git’s error after the startup finish fails', async () => { + it('keeps the leftover listed with the locked file’s error after the startup finish fails', async () => { const error = await failStartupFinish() - expect(String(error)).toMatch(/Operation not permitted/) + expect(String(error)).toMatch(LOCKED_FILE_ERROR) // Git removed registration and merged branch but left checkout files and Orca metadata. expect(await isRegistered(worktreePath)).toBe(false) expect(existsSync(lockedFile)).toBe(true) expect(await git(['branch', '--list', 'feature'])).toBe('') expect(await listedRows()).toEqual([ - { path: worktreePath, removalError: expect.stringMatching(/Operation not permitted/) } + { + path: worktreePath, + removalError: expect.stringMatching(LOCKED_FILE_ERROR) + } ]) // The leftover is not fenced: terminals may open in it while it waits for the user. beginTerminalInstall(worktreePath)() @@ -385,7 +390,7 @@ describe.skipIf(process.platform !== 'darwin')('a worktree delete Git fails part ) await _settlePendingWorktreeRemovalsForTests() - expect(String(error)).toMatch(/Operation not permitted/) + expect(String(error)).toMatch(LOCKED_FILE_ERROR) expect(await git(['remote'])).not.toContain(pushTarget.remoteName) expect(existsSync(lockedFile)).toBe(true) }) @@ -393,7 +398,7 @@ describe.skipIf(process.platform !== 'darwin')('a worktree delete Git fails part it('removes the push-target remote Orca added when the startup finish fails', async () => { const pushTarget = await addOrcaPushTargetRemote() - expect(String(await finishAtStartup([], pushTarget))).toMatch(/Operation not permitted/) + expect(String(await finishAtStartup([], pushTarget))).toMatch(LOCKED_FILE_ERROR) expect(await git(['remote'])).not.toContain(pushTarget.remoteName) expect(existsSync(lockedFile)).toBe(true) diff --git a/src/main/runtime/runtime-registered-local-worktree-removal.ts b/src/main/runtime/runtime-registered-local-worktree-removal.ts index 515068dcdbf..8a2255560e3 100644 --- a/src/main/runtime/runtime-registered-local-worktree-removal.ts +++ b/src/main/runtime/runtime-registered-local-worktree-removal.ts @@ -26,7 +26,7 @@ import { isOrphanedWorktreeError } from '../ipc/worktree-logic' import { cleanupLocalOrphanedWorktreeDirectory } from '../local-orphaned-worktree-cleanup' -import { recoverLocalWindowsWorktreeRemoval } from '../local-worktree-removal-recovery' +import { recoverPartialLocalWorktreeRemoval } from '../local-worktree-removal-recovery' import { findRegisteredDeletableWorktree } from '../worktree-removal-safety' import { CLIENT_REMOVAL_HOME } from '../worktree-removal-home-guard' import type { RuntimeStore } from './runtime-store-contract' @@ -249,7 +249,7 @@ export async function finishRuntimeLocalWorktreeRemoval( if (usesNativeWindowsRemoval(canonicalPath, localOptions)) { throw error } - const recovered = await recoverLocalWindowsWorktreeRemoval({ + const recovered = await recoverPartialLocalWorktreeRemoval({ error, force: args.force, canonicalWorktreePath: canonicalPath,