From bb40dcd0286bc64bfa254c5f7f3faaebd4c7b1f5 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sun, 7 Jun 2026 11:01:26 -0700 Subject: [PATCH] fix: recover missing worktree deletes (#4811) --- src/main/ipc/worktrees.test.ts | 27 +++++++++++++++++-- src/main/ipc/worktrees.ts | 13 +++++---- src/main/runtime/orca-runtime.ts | 8 +++++- src/main/worktree-removal-safety.ts | 2 ++ .../sidebar/delete-worktree-toast.test.ts | 14 ++++++++++ .../sidebar/delete-worktree-toast.ts | 9 +++++++ .../src/store/slices/store-cascades.test.ts | 27 +++++++++++++++++++ src/renderer/src/store/slices/worktrees.ts | 3 ++- 8 files changed, 94 insertions(+), 9 deletions(-) diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index fa2ecde8a..1288591c7 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -4320,7 +4320,7 @@ describe('registerWorktreeHandlers', () => { }) }) - it('rejects unregistered delete paths before teardown, hooks, or git removal', async () => { + it('reports already-missing unregistered delete paths before teardown, hooks, or git removal', async () => { mockKnownFeatureWorktree('/workspace/real-feature') getEffectiveHooksMock.mockReturnValue({ scripts: { @@ -4332,7 +4332,9 @@ describe('registerWorktreeHandlers', () => { handlers['worktrees:remove'](null, { worktreeId: 'repo-1::/workspace/not-a-worktree' }) - ).rejects.toThrow('Refusing to delete unregistered worktree path') + ).rejects.toThrow( + 'Worktree is no longer registered with Git and its directory is already gone.' + ) expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled() expect(runHookMock).not.toHaveBeenCalled() @@ -4363,6 +4365,27 @@ describe('registerWorktreeHandlers', () => { }) }) + it('cleans up an already-missing unregistered worktree after force recovery', async () => { + const worktreeId = 'repo-1::/workspace/already-deleted-wt' + mockKnownFeatureWorktree('/workspace/real-feature') + + await expect(handlers['worktrees:remove'](null, { worktreeId })).rejects.toThrow( + 'Worktree is no longer registered with Git and its directory is already gone.' + ) + + await handlers['worktrees:remove'](null, { worktreeId, force: true }) + + expect(killAllProcessesForWorktreeMock).not.toHaveBeenCalled() + expect(runHookMock).not.toHaveBeenCalled() + expect(removeWorktreeMock).not.toHaveBeenCalled() + expect(runtimeStub.clearOptimisticReconcileToken).toHaveBeenCalledWith(worktreeId) + expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) + expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(worktreeId) + expect(mainWindow.webContents.send).toHaveBeenCalledWith('worktrees:changed', { + repoId: 'repo-1' + }) + }) + it('treats normal deletion of an already-missing unregistered worktree as cleanup', async () => { mockKnownFeatureWorktree('/workspace/real-feature') store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta()) diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 3d91cb934..f8303ae60 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -90,7 +90,8 @@ import { findRegisteredDeletableWorktree, isWorktreePathMissing, ORPHANED_WORKTREE_DIRECTORY_MESSAGE, - stripOrcaProvenanceMetaUpdates + stripOrcaProvenanceMetaUpdates, + UNREGISTERED_MISSING_WORKTREE_MESSAGE } from '../worktree-removal-safety' import { isWindowsAbsolutePathLike } from '../../shared/cross-platform-path' import { DEFAULT_WORKSPACE_STATUS_ID } from '../../shared/workspace-statuses' @@ -1123,10 +1124,12 @@ export function registerWorktreeHandlers( notifyWorktreesChanged(mainWindow, repoId) return {} } - if ( - (args.force || removedMeta) && - (await isAlreadyRemovedWorktreePath(repo, worktreePath)) - ) { + if (await isAlreadyRemovedWorktreePath(repo, worktreePath)) { + if (!args.force && !removedMeta) { + // Why: without persisted metadata, require the renderer recovery + // path before deleting Orca-only state for an unregistered path. + throw new Error(UNREGISTERED_MISSING_WORKTREE_MESSAGE) + } // Why: a manually deleted worktree is already gone from Git and disk. // The sidebar delete action has persisted metadata proving this was // an Orca-known row, so no force confirmation is needed. diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index d3af8b734..aede15959 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -444,7 +444,8 @@ import { findRegisteredDeletableWorktree, isWorktreePathMissing, ORPHANED_WORKTREE_DIRECTORY_MESSAGE, - stripOrcaProvenanceMetaUpdates + stripOrcaProvenanceMetaUpdates, + UNREGISTERED_MISSING_WORKTREE_MESSAGE } from '../worktree-removal-safety' import { prefetchWorktreeCreateBase } from '../worktree-create-base-prefetch' import { invalidateAuthorizedRootsCache } from '../ipc/filesystem-auth' @@ -10353,6 +10354,11 @@ export class OrcaRuntimeService { return {} } if (await isRuntimeWorktreePathMissing(repo, removalTarget.path)) { + if (!force && !removedMeta) { + // Why: without persisted metadata, require the renderer recovery + // path before deleting Orca-only state for an unregistered path. + throw new Error(UNREGISTERED_MISSING_WORKTREE_MESSAGE) + } // Why: a manually deleted worktree is already gone from Git and disk. // Finish runtime metadata cleanup without requiring force or touching // any unregistered path that still exists. diff --git a/src/main/worktree-removal-safety.ts b/src/main/worktree-removal-safety.ts index 32ae52435..a798b219e 100644 --- a/src/main/worktree-removal-safety.ts +++ b/src/main/worktree-removal-safety.ts @@ -38,6 +38,8 @@ type UnregisteredOrcaCleanupMeta = Pick< export const ORPHANED_WORKTREE_DIRECTORY_MESSAGE = 'Worktree is no longer registered with Git but its directory remains.' +export const UNREGISTERED_MISSING_WORKTREE_MESSAGE = + 'Worktree is no longer registered with Git and its directory is already gone.' function getPathOps(...paths: string[]): PathOps { // Why: forward-slash UNC roots need win32 ops; POSIX joins collapse `//Server` to `/Server`. diff --git a/src/renderer/src/components/sidebar/delete-worktree-toast.test.ts b/src/renderer/src/components/sidebar/delete-worktree-toast.test.ts index da46e64fd..a13c4eadf 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-toast.test.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-toast.test.ts @@ -25,6 +25,20 @@ describe('getDeleteWorktreeToastCopy', () => { }) }) + it('uses stale-row guidance when Git already removed the worktree directory', () => { + expect( + getDeleteWorktreeToastCopy( + 'feature/foo', + true, + 'Worktree is no longer registered with Git and its directory is already gone.' + ) + ).toEqual({ + title: 'Failed to delete workspace feature/foo', + description: 'Git already removed this workspace. Use Force Delete to clear it from Orca.', + isDestructive: false + }) + }) + it('preserves the raw error when force delete is unavailable', () => { expect(getDeleteWorktreeToastCopy('feature/foo', false, 'permission denied')).toEqual({ title: 'Failed to delete workspace feature/foo', diff --git a/src/renderer/src/components/sidebar/delete-worktree-toast.ts b/src/renderer/src/components/sidebar/delete-worktree-toast.ts index 993db17aa..c794a66b0 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-toast.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-toast.ts @@ -18,6 +18,15 @@ export function getDeleteWorktreeToastCopy( isDestructive: false } } + if ( + error.includes('Worktree is no longer registered with Git and its directory is already gone.') + ) { + return { + title: `Failed to delete workspace ${worktreeName}`, + description: 'Git already removed this workspace. Use Force Delete to clear it from Orca.', + isDestructive: false + } + } return { title: `Failed to delete workspace ${worktreeName}`, description: 'It has changed files. Use Force Delete to delete it anyway.', diff --git a/src/renderer/src/store/slices/store-cascades.test.ts b/src/renderer/src/store/slices/store-cascades.test.ts index 15f64b127..7186d3b0e 100644 --- a/src/renderer/src/store/slices/store-cascades.test.ts +++ b/src/renderer/src/store/slices/store-cascades.test.ts @@ -320,6 +320,33 @@ describe('removeWorktree cascade', () => { }) }) + it('offers force delete when Git already removed an unregistered worktree', async () => { + const store = createTestStore() + const worktreeId = 'repo1::/workspace/deleted-wt' + const error = + "Error invoking remote method 'worktrees:remove': Error: Worktree is no longer registered with Git and its directory is already gone." + + mockApi.worktrees.remove.mockRejectedValueOnce(new Error(error)) + + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: worktreeId, repoId: 'repo1' })] + }, + tabsByWorktree: {}, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: {} + }) + + const result = await store.getState().removeWorktree(worktreeId) + + expect(result).toEqual({ ok: false, error }) + expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({ + isDeleting: false, + error, + canForceDelete: true + }) + }) + it('sets canForceDelete=false when force=true removal fails', async () => { const store = createTestStore() const worktreeId = 'repo1::/path/wt1' diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index e91d34fc6..8bf09d52f 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -237,7 +237,8 @@ function toVisibleTabType(contentType: string): WorkspaceVisibleTabType { const FORCE_RETRYABLE_WORKTREE_REMOVAL_MESSAGES = [ 'Worktree has uncommitted or untracked changes', 'contains modified or untracked files', - 'Worktree is no longer registered with Git but its directory remains' + 'Worktree is no longer registered with Git but its directory remains', + 'Worktree is no longer registered with Git and its directory is already gone' ] as const // Why: local preflight formatting can surface raw git porcelain instead of the