diff --git a/src/main/git/remove-worktree.test.ts b/src/main/git/remove-worktree.test.ts index 991cd382d75..53c424e50f6 100644 --- a/src/main/git/remove-worktree.test.ts +++ b/src/main/git/remove-worktree.test.ts @@ -272,40 +272,6 @@ branch refs/heads/main expect(getGitCalls()).toContain('git worktree remove --force /repo-feature') }) - it('rejects lock override without dirty-file force before invoking Git', async () => { - await expect( - removeWorktree('/repo', '/repo-feature', false, { overrideLock: true }) - ).rejects.toThrow('Worktree lock override requires force deletion permission.') - - expect(getGitCalls()).toEqual([]) - }) - - it('passes double --force only for explicit lock override', 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 -locked active agent session -` - }, - 'git worktree list --porcelain#2': { - stdout: `worktree /repo -HEAD abc123 -branch refs/heads/main -` - } - }) - - await removeWorktree('/repo', '/repo-feature', true, { overrideLock: true }) - - expect(getGitCalls()).toContain('git worktree remove --force --force /repo-feature') - }) - it('rejects a locked worktree with stable app-owned copy before invoking remove', async () => { mockGitCommands({ 'git worktree list --porcelain': { @@ -321,7 +287,7 @@ locked active agent session } }) - await expect(removeWorktree('/repo', '/repo-feature')).rejects.toThrow( + await expect(removeWorktree('/repo', '/repo-feature', true)).rejects.toThrow( 'Worktree is locked by Git. Lock reason: active agent session.' ) expect(getGitCalls()).not.toContain('git worktree remove /repo-feature') diff --git a/src/main/git/worktree-list-paths.test.ts b/src/main/git/worktree-list-paths.test.ts index 3602fa75097..df83417e7cb 100644 --- a/src/main/git/worktree-list-paths.test.ts +++ b/src/main/git/worktree-list-paths.test.ts @@ -102,14 +102,16 @@ describe('git worktree paths', () => { } ) - it('force-removes a locked worktree whose directory was deleted manually', async () => { + it('preserves a locked worktree whose directory was deleted manually', async () => { const { repoPath, worktreePath } = await createRepoWithLockedDeletedWorktree() - await expect( - removeWorktree(repoPath, worktreePath, true, { overrideLock: true }) - ).resolves.toEqual({}) + await expect(removeWorktree(repoPath, worktreePath, true)).rejects.toThrow( + 'Worktree is locked by Git' + ) - expect(git(repoPath, ['worktree', 'list', '--porcelain'])).not.toContain(worktreePath) - expect(branchExists(repoPath, 'feature/locked-delete')).toBe(false) + expect(git(repoPath, ['worktree', 'list', '--porcelain'])).toContain( + worktreePath.replaceAll('\\', '/') + ) + expect(branchExists(repoPath, 'feature/locked-delete')).toBe(true) }) }) diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index e6194b05109..11961404fb1 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -13,10 +13,7 @@ import type { LocalBaseRefUpdateSuggestion, RemoveWorktreeResult } from '../../shared/types' -import { - assertWorktreeRemovalForcePermissions, - assertWorktreeUnlockedForRemoval -} from '../../shared/worktree-removal' +import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree-removal' import { decodeGitCQuotedPath } from '../../shared/git-cquoted-path' import { parseGitRevListAheadBehindCounts } from '../../shared/git-rev-list-output' import { parseWslUncPath } from '../../shared/wsl-paths' @@ -51,7 +48,6 @@ export type AddWorktreeOptions = GitWorktreeExecOptions & { export type RemoveWorktreeOptions = GitWorktreeExecOptions & { deleteBranch?: boolean forceBranchDelete?: boolean - overrideLock?: boolean knownRemovedWorktree?: Pick } @@ -1080,7 +1076,6 @@ async function performRemoveWorktree( force = false, options: RemoveWorktreeOptions = {} ): Promise { - assertWorktreeRemovalForcePermissions(force, options.overrideLock) const removedWorktree = options.knownRemovedWorktree ?? (await listWorktrees(repoPath, options)).find((worktree) => @@ -1091,14 +1086,10 @@ async function performRemoveWorktree( // Why: callers outside the IPC/runtime preflight must not bypass Git's lock // contract or depend on localized stderr to discover it after side effects. - assertWorktreeUnlockedForRemoval(removedWorktree, options.overrideLock) + assertWorktreeUnlockedForRemoval(removedWorktree) const args = ['worktree', 'remove'] - if (options.overrideLock) { - // Why: Git reserves the second force for explicitly overriding a lock; - // ordinary dirty-file confirmation must not silently break that lock. - args.push('--force', '--force') - } else if (force) { + if (force) { args.push('--force') } args.push(worktreePath) diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index e6447cca047..23ab06c335c 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -6402,7 +6402,7 @@ describe('registerWorktreeHandlers', () => { expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) }) - it('removes a locked missing registration only with explicit lock override', async () => { + it('preserves a locked missing registration even with force', async () => { setPlatform('win32') const missingWorktreePath = 'C:\\workspace\\locked-already-removed' const worktreeId = `repo-1::${missingWorktreePath}` @@ -6424,26 +6424,16 @@ describe('registerWorktreeHandlers', () => { lockReason: 'active agent session' } ] - listWorktreesMock.mockResolvedValueOnce(registeredWorktrees).mockResolvedValue([]) + listWorktreesMock.mockResolvedValue(registeredWorktrees) store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta()) removeWorktreeMock.mockResolvedValue({}) - await handlers['worktrees:remove'](null, { - worktreeId, - force: true, - overrideLock: true - }) - - expect(removeWorktreeMock).toHaveBeenCalledWith( - '/workspace/repo', - missingWorktreePath, - true, - expect.objectContaining({ - overrideLock: true, - knownRemovedWorktree: expect.objectContaining({ locked: true }) - }) + await expect(handlers['worktrees:remove'](null, { worktreeId, force: true })).rejects.toThrow( + 'Worktree is locked by Git. Lock reason: active agent session' ) - expect(store.removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) + + expect(removeWorktreeMock).not.toHaveBeenCalled() + expect(store.removeWorktreeMeta).not.toHaveBeenCalled() }) it('refuses to delete the root workspace for folder-mode repos', async () => { @@ -6748,14 +6738,11 @@ describe('registerWorktreeHandlers', () => { await handlers['worktrees:remove'](null, { worktreeId: 'repo-ssh::/remote/feature-wt', - force: true, - overrideLock: true + force: true }) expect(provider.worktreeIsClean).not.toHaveBeenCalled() - expect(provider.removeWorktree).toHaveBeenCalledWith('/remote/feature-wt', true, { - overrideLock: true - }) + expect(provider.removeWorktree).toHaveBeenCalledWith('/remote/feature-wt', true) }) it('continues SSH worktree removal when the archive hook fails', async () => { @@ -7798,19 +7785,6 @@ describe('registerWorktreeHandlers', () => { expect(removeWorktreeMock).not.toHaveBeenCalled() }) - it('rejects lock override without dirty-file force before any removal work', async () => { - await expect( - handlers['worktrees:remove'](null, { - worktreeId: 'repo-1::/workspace/feature-wt', - overrideLock: true - }) - ).rejects.toThrow('Worktree lock override requires force deletion permission.') - - expect(listWorktreesMock).not.toHaveBeenCalled() - expect(runHookMock).not.toHaveBeenCalled() - expect(removeWorktreeMock).not.toHaveBeenCalled() - }) - it('fails locked dirty-force deletes before hooks, link cleanup, or PTY teardown', async () => { listWorktreesMock.mockResolvedValue([ { diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index 8735948a59e..afe85c1f9e4 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -29,10 +29,7 @@ import type { Worktree, WorktreeMeta } from '../../shared/types' -import { - assertWorktreeRemovalForcePermissions, - assertWorktreeUnlockedForRemoval -} from '../../shared/worktree-removal' +import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree-removal' import { buildKnownOrcaWorkspaceLayouts, isLegacyRepoForExternalWorktreeVisibility, @@ -298,15 +295,10 @@ function gitStatusErrorMeansNotRepository(error: unknown): boolean { return /not a git repository/i.test(`${message}\n${stderr}`) } -function getWorktreeRemovalOptionsKey(args: { - force?: boolean - overrideLock?: boolean - skipArchive?: boolean -}): string { +function getWorktreeRemovalOptionsKey(args: { force?: boolean; skipArchive?: boolean }): string { const forceKey = args.force === true ? 'force' : 'normal' - const lockKey = args.overrideLock === true ? 'override-lock' : 'respect-lock' const archiveKey = args.skipArchive === true ? 'skip-archive' : 'run-archive' - return `${forceKey}:${lockKey}:${archiveKey}` + return `${forceKey}:${archiveKey}` } async function getArchiveHooksForRemoval(repo: Repo): Promise { @@ -1359,11 +1351,9 @@ export function registerWorktreeHandlers( args: { worktreeId: string force?: boolean - overrideLock?: boolean skipArchive?: boolean } ) => { - assertWorktreeRemovalForcePermissions(args.force === true, args.overrideLock === true) const optionsKey = getWorktreeRemovalOptionsKey(args) const inFlightRemoval = worktreeRemovalsInFlight.get(args.worktreeId) if (inFlightRemoval) { @@ -1566,7 +1556,7 @@ export function registerWorktreeHandlers( // Why: a Git lock must block before archive hooks or linked-path cleanup // mutate the workspace; dirty-file force is a separate permission. try { - assertWorktreeUnlockedForRemoval(registeredWorktree, args.overrideLock ?? false) + assertWorktreeUnlockedForRemoval(registeredWorktree) } catch (error) { throw new Error( formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false) @@ -1589,8 +1579,7 @@ export function registerWorktreeHandlers( repoPath: repo.path, localWorktreeGitOptions, registeredWorktree, - deleteBranch, - overrideLock: args.overrideLock === true + deleteBranch }) await cleanupUnusedWorktreePushTargetRemote( repo.path, @@ -1646,10 +1635,7 @@ export function registerWorktreeHandlers( } } - const remoteRemoveOptions = { - ...(!deleteBranch ? { deleteBranch } : {}), - ...(args.overrideLock ? { overrideLock: true } : {}) - } + const remoteRemoveOptions = !deleteBranch ? { deleteBranch } : {} const rawRemovalResult = await (Object.keys(remoteRemoveOptions).length > 0 ? provider!.removeWorktree(canonicalWorktreePath, args.force, remoteRemoveOptions) : provider!.removeWorktree(canonicalWorktreePath, args.force)) @@ -1692,7 +1678,7 @@ export function registerWorktreeHandlers( try { // Why: an archive hook can race another Git client that locks the row; // recheck before linked-path, watcher, or terminal teardown side effects. - assertWorktreeUnlockedForRemoval(refreshedRegisteredWorktree, args.overrideLock ?? false) + assertWorktreeUnlockedForRemoval(refreshedRegisteredWorktree) } catch (error) { throw new Error( formatWorktreeRemovalError(error, canonicalWorktreePath, args.force ?? false) @@ -1753,7 +1739,6 @@ export function registerWorktreeHandlers( try { const removeOptions = { ...(!deleteBranch ? { deleteBranch } : {}), - ...(args.overrideLock ? { overrideLock: true } : {}), // Why: this handler already paid for an authoritative worktree // list to validate the target; reuse it instead of rescanning // every sibling worktree during the hot delete path. @@ -1780,7 +1765,6 @@ export function registerWorktreeHandlers( localWorktreeGitOptions, registeredWorktree: refreshedRegisteredWorktree, deleteBranch, - overrideLock: args.overrideLock === true, closeWatcher: closeLocalWatcherForRemoval }) if (recoveredRemovalResult) { diff --git a/src/main/local-worktree-removal-recovery-live.test.ts b/src/main/local-worktree-removal-recovery-live.test.ts index 7c7978be000..4d5a048b5f4 100644 --- a/src/main/local-worktree-removal-recovery-live.test.ts +++ b/src/main/local-worktree-removal-recovery-live.test.ts @@ -115,7 +115,6 @@ describe('local Windows worktree removal recovery (live Git)', () => { localWorktreeGitOptions: {}, registeredWorktree: { branch: 'refs/heads/feature/churn', head }, deleteBranch: false, - overrideLock: false, closeWatcher: async () => {} }) ).resolves.toEqual({}) diff --git a/src/main/local-worktree-removal-recovery.test.ts b/src/main/local-worktree-removal-recovery.test.ts index aec47789aba..7c46727dae1 100644 --- a/src/main/local-worktree-removal-recovery.test.ts +++ b/src/main/local-worktree-removal-recovery.test.ts @@ -1,16 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { - gitExecFileAsyncMock, - listWorktreesStrictMock, - removeLocalWorktreePathMock, - removeWorktreeMock -} = vi.hoisted(() => ({ - gitExecFileAsyncMock: vi.fn(), - listWorktreesStrictMock: vi.fn(), - removeLocalWorktreePathMock: vi.fn(), - removeWorktreeMock: vi.fn() -})) +const { gitExecFileAsyncMock, listWorktreesStrictMock, removeLocalWorktreePathMock } = vi.hoisted( + () => ({ + gitExecFileAsyncMock: vi.fn(), + listWorktreesStrictMock: vi.fn(), + removeLocalWorktreePathMock: vi.fn() + }) +) vi.mock('./git/runner', () => ({ gitExecFileAsync: gitExecFileAsyncMock @@ -21,8 +17,7 @@ vi.mock('./local-worktree-filesystem', () => ({ })) vi.mock('./git/worktree', () => ({ - listWorktreesStrict: listWorktreesStrictMock, - removeWorktree: removeWorktreeMock + listWorktreesStrict: listWorktreesStrictMock })) import { @@ -45,11 +40,9 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { gitExecFileAsyncMock.mockReset() listWorktreesStrictMock.mockReset() removeLocalWorktreePathMock.mockReset() - removeWorktreeMock.mockReset() gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' }) listWorktreesStrictMock.mockResolvedValue([]) removeLocalWorktreePathMock.mockResolvedValue(undefined) - removeWorktreeMock.mockResolvedValue({}) }) afterEach(() => { @@ -70,7 +63,6 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { localWorktreeGitOptions: {}, registeredWorktree: { branch: 'refs/heads/delete-e2e-held-cwd', head: 'abc123' }, deleteBranch: true, - overrideLock: false, closeWatcher: vi.fn().mockResolvedValue(undefined) }) @@ -105,7 +97,6 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { localWorktreeGitOptions: {}, registeredWorktree: { branch: 'refs/heads/delete-e2e-held-cwd', head: 'abc123' }, deleteBranch: true, - overrideLock: false, closeWatcher: vi.fn().mockResolvedValue(undefined) }) @@ -137,7 +128,6 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { localWorktreeGitOptions: {}, registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' }, deleteBranch: false, - overrideLock: false, closeWatcher: vi.fn().mockResolvedValue(undefined) }) @@ -169,7 +159,6 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { localWorktreeGitOptions: {}, registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' }, deleteBranch: false, - overrideLock: false, closeWatcher: vi.fn().mockResolvedValue(undefined) }) ).resolves.toBeUndefined() @@ -192,7 +181,6 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { localWorktreeGitOptions: {}, registeredWorktree: { branch: 'refs/heads/delete-e2e-held-cwd', head: 'abc123' }, deleteBranch: true, - overrideLock: false, closeWatcher: vi.fn().mockResolvedValue(undefined) }) ).resolves.toBeUndefined() @@ -200,7 +188,7 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { }) }) - it('uses explicit lock override when long-path cleanup leaves a locked registration', async () => { + it('does not recover a locked registration after a filesystem removal failure', async () => { await withPlatform('win32', async () => { const registeredWorktree = { branch: 'refs/heads/feature', @@ -209,29 +197,22 @@ describe('recoverLocalWindowsWorktreeRemoval', () => { lockReason: 'active agent' } - await recoverLocalWindowsWorktreeRemoval({ - error: Object.assign(new Error('git worktree remove failed'), { - stderr: 'error: failed to delete deep/file.txt: Filename too long' - }), - force: true, - canonicalWorktreePath: 'C:/workspaces/feature', - repoPath: 'C:/repo', - localWorktreeGitOptions: {}, - registeredWorktree, - deleteBranch: true, - overrideLock: true, - closeWatcher: vi.fn().mockResolvedValue(undefined) - }) - - expect(removeWorktreeMock).toHaveBeenCalledWith( - 'C:/repo', - 'C:/workspaces/feature', - true, - expect.objectContaining({ - overrideLock: true, - knownRemovedWorktree: registeredWorktree + await expect( + recoverLocalWindowsWorktreeRemoval({ + error: Object.assign(new Error('git worktree remove failed'), { + stderr: 'error: failed to delete deep/file.txt: Filename too long' + }), + force: true, + canonicalWorktreePath: 'C:/workspaces/feature', + repoPath: 'C:/repo', + localWorktreeGitOptions: {}, + registeredWorktree, + deleteBranch: true, + closeWatcher: vi.fn().mockResolvedValue(undefined) }) - ) + ).rejects.toThrow('Worktree is locked by Git. Lock reason: active agent') + + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() }) }) }) @@ -240,39 +221,11 @@ describe('removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval', () => { beforeEach(() => { gitExecFileAsyncMock.mockReset() listWorktreesStrictMock.mockReset() - removeWorktreeMock.mockReset() gitExecFileAsyncMock.mockResolvedValue({ stdout: '', stderr: '' }) listWorktreesStrictMock.mockResolvedValue([]) - removeWorktreeMock.mockResolvedValue({}) }) - it('uses explicit double-force removal for a locked missing registration', async () => { - const registeredWorktree = { - branch: 'refs/heads/feature', - head: 'abc123', - locked: true, - lockReason: 'active agent' - } - - await removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({ - canonicalWorktreePath: 'C:/workspaces/feature', - repoPath: 'C:/repo', - localWorktreeGitOptions: {}, - registeredWorktree, - deleteBranch: false, - overrideLock: true - }) - - expect(gitExecFileAsyncMock).not.toHaveBeenCalledWith(['worktree', 'prune'], expect.anything()) - expect(removeWorktreeMock).toHaveBeenCalledWith('C:/repo', 'C:/workspaces/feature', true, { - deleteBranch: false, - overrideLock: true, - knownRemovedWorktree: registeredWorktree - }) - expect(listWorktreesStrictMock).toHaveBeenCalledWith('C:/repo', {}) - }) - - it('does not override a locked missing registration without explicit permission', async () => { + it('does not override a locked missing registration', async () => { await expect( removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval({ canonicalWorktreePath: 'C:/workspaces/feature', @@ -284,12 +237,10 @@ describe('removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval', () => { locked: true, lockReason: 'active agent' }, - deleteBranch: true, - overrideLock: false + deleteBranch: true }) ).rejects.toThrow('Worktree is locked by Git') - expect(removeWorktreeMock).not.toHaveBeenCalled() expect(gitExecFileAsyncMock).not.toHaveBeenCalled() }) @@ -310,8 +261,7 @@ describe('removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval', () => { repoPath: 'C:/repo', localWorktreeGitOptions: {}, registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' }, - deleteBranch: true, - overrideLock: false + deleteBranch: true }) ).rejects.toThrow('Git still has stale worktree registration') }) diff --git a/src/main/local-worktree-removal-recovery.ts b/src/main/local-worktree-removal-recovery.ts index f4d67ec96e8..7ea73ea413e 100644 --- a/src/main/local-worktree-removal-recovery.ts +++ b/src/main/local-worktree-removal-recovery.ts @@ -6,7 +6,7 @@ import { isWindowsLongPathWorktreeRemovalError } from './ipc/worktree-logic' import { gitExecFileAsync } from './git/runner' -import { listWorktreesStrict, removeWorktree, type GitWorktreeExecOptions } from './git/worktree' +import { listWorktreesStrict, type GitWorktreeExecOptions } from './git/worktree' import { removeLocalWorktreePath } from './local-worktree-filesystem' type LocalWindowsRemovalRecoveryArgs = { @@ -17,7 +17,6 @@ type LocalWindowsRemovalRecoveryArgs = { localWorktreeGitOptions: GitWorktreeExecOptions registeredWorktree: Pick deleteBranch: boolean - overrideLock: boolean closeWatcher: (worktreePath: string) => Promise } @@ -79,27 +78,16 @@ async function removeRequiredGitWorktreeRegistration( args: StaleLocalWorktreeRegistrationArgs, forceForError = true ): Promise { - assertWorktreeUnlockedForRemoval(args.registeredWorktree, args.overrideLock) + assertWorktreeUnlockedForRemoval(args.registeredWorktree) let result: RemoveWorktreeResult | undefined let removalError: unknown try { - if (args.registeredWorktree.locked && args.overrideLock) { - // Why: prune intentionally retains locked rows; Git's second force is the - // explicit recovery mechanism that can remove the missing registration. - result = await removeWorktree(args.repoPath, args.canonicalWorktreePath, true, { - ...args.localWorktreeGitOptions, - deleteBranch: args.deleteBranch, - overrideLock: true, - knownRemovedWorktree: args.registeredWorktree - }) - } else { - await gitExecFileAsync(['worktree', 'prune'], { - cwd: args.repoPath, - ...args.localWorktreeGitOptions - }) - result = preservedBranchResult(args.registeredWorktree, args.deleteBranch) - } + await gitExecFileAsync(['worktree', 'prune'], { + cwd: args.repoPath, + ...args.localWorktreeGitOptions + }) + result = preservedBranchResult(args.registeredWorktree, args.deleteBranch) } catch (error) { removalError = error } diff --git a/src/main/providers/ssh-git-provider.test.ts b/src/main/providers/ssh-git-provider.test.ts index 5397069b4e4..acfaefaed12 100644 --- a/src/main/providers/ssh-git-provider.test.ts +++ b/src/main/providers/ssh-git-provider.test.ts @@ -1211,11 +1211,10 @@ describe('SshGitProvider', () => { }) it('removeWorktree sends git.removeWorktree request', async () => { - await provider.removeWorktree('/home/user/feat', true, { overrideLock: true }) + await provider.removeWorktree('/home/user/feat', true) expect(mux.request).toHaveBeenCalledWith('git.removeWorktree', { worktreePath: '/home/user/feat', - force: true, - overrideLock: true + force: true }) }) diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index 9f205f9cd15..5ba6a29e380 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -656,7 +656,7 @@ export class SshGitProvider implements IGitProvider { async removeWorktree( worktreePath: string, force?: boolean, - options?: { deleteBranch?: boolean; forceBranchDelete?: boolean; overrideLock?: boolean } + options?: { deleteBranch?: boolean; forceBranchDelete?: boolean } ): Promise { return this.runWithDiffDedupeClear( async () => diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index 4ace4db9a01..c8e2680366b 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -286,7 +286,7 @@ export type IGitProvider = { removeWorktree( worktreePath: string, force?: boolean, - options?: { deleteBranch?: boolean; forceBranchDelete?: boolean; overrideLock?: boolean } + options?: { deleteBranch?: boolean; forceBranchDelete?: boolean } ): Promise renameCurrentBranch?(worktreePath: string, newBranch: string): Promise forceDeletePreservedBranch?( diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index e8e4a3d0a95..7e4ad5d94dc 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -4096,14 +4096,12 @@ describe('OrcaRuntimeService', () => { const runtime = new OrcaRuntimeService(remoteStore as never) try { - await runtime.removeManagedWorktree('path:/remote/feature', true, false, true) + await runtime.removeManagedWorktree('path:/remote/feature', true, false) } finally { unregisterSshGitProvider('ssh-1') } - expect(gitProvider.removeWorktree).toHaveBeenCalledWith('/remote/feature', true, { - overrideLock: true - }) + expect(gitProvider.removeWorktree).toHaveBeenCalledWith('/remote/feature', true) expect(removeWorktree).not.toHaveBeenCalled() expect(listWorktrees).not.toHaveBeenCalled() expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(`${TEST_REPO_ID}::/remote/feature`) @@ -26000,7 +25998,7 @@ describe('OrcaRuntimeService', () => { } }) - it('removes a locked missing runtime registration only with explicit lock override', async () => { + it('preserves a locked missing runtime registration even with force', async () => { setPlatform('win32') const missingWorktreePath = 'C:\\workspace\\locked-already-removed' const worktreeId = `${TEST_REPO_ID}::${missingWorktreePath}` @@ -26017,21 +26015,15 @@ describe('OrcaRuntimeService', () => { lockReason: 'active agent session' } ] - vi.mocked(listWorktreesStrict).mockResolvedValueOnce(registeredWorktrees).mockResolvedValue([]) + vi.mocked(listWorktreesStrict).mockResolvedValue(registeredWorktrees) vi.mocked(removeWorktree).mockResolvedValue({}) - await runtime.removeManagedWorktree(worktreeId, true, false, true) - - expect(removeWorktree).toHaveBeenCalledWith( - TEST_REPO_PATH, - missingWorktreePath, - true, - expect.objectContaining({ - overrideLock: true, - knownRemovedWorktree: expect.objectContaining({ locked: true }) - }) + await expect(runtime.removeManagedWorktree(worktreeId, true, false)).rejects.toThrow( + 'Worktree is locked by Git. Lock reason: active agent session' ) - expect(removeWorktreeMeta).toHaveBeenCalledWith(worktreeId) + + expect(removeWorktree).not.toHaveBeenCalled() + expect(removeWorktreeMeta).not.toHaveBeenCalled() }) it('routes runtime worktree removal through the selected WSL project runtime', async () => { @@ -26762,21 +26754,6 @@ describe('OrcaRuntimeService', () => { expect(removeWorktree).not.toHaveBeenCalled() }) - it('rejects lock override without dirty-file force before any removal work', async () => { - const runtime = new OrcaRuntimeService(store) - const listCallCount = vi.mocked(listWorktreesStrict).mock.calls.length - const hookCallCount = vi.mocked(runHook).mock.calls.length - const removeCallCount = vi.mocked(removeWorktree).mock.calls.length - - await expect( - runtime.removeManagedWorktree(TEST_WORKTREE_ID, false, false, true) - ).rejects.toThrow('Worktree lock override requires force deletion permission.') - - expect(vi.mocked(listWorktreesStrict).mock.calls).toHaveLength(listCallCount) - expect(vi.mocked(runHook).mock.calls).toHaveLength(hookCallCount) - expect(vi.mocked(removeWorktree).mock.calls).toHaveLength(removeCallCount) - }) - it('fails locked dirty-force deletes before hooks, link cleanup, or PTY teardown', async () => { const repo = { ...store.getRepos()[0], symlinkPaths: ['node_modules'] } const runtimeStore = { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 36f8d047c23..a3eac592ed6 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -127,10 +127,7 @@ import type { WorkspaceSessionState, DirEntry } from '../../shared/types' -import { - assertWorktreeRemovalForcePermissions, - assertWorktreeUnlockedForRemoval -} from '../../shared/worktree-removal' +import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree-removal' import { getRepoExecutionHostId, parseExecutionHostId, @@ -1427,12 +1424,8 @@ type PreservedBranchCleanupTarget = { pushTarget?: GitPushTarget } -function getRuntimeWorktreeRemovalOptionsKey( - force: boolean, - runHooks: boolean, - overrideLock: boolean -): string { - return `${force ? 'force' : 'normal'}:${overrideLock ? 'override-lock' : 'respect-lock'}:${runHooks ? 'run-hooks' : 'skip-hooks'}` +function getRuntimeWorktreeRemovalOptionsKey(force: boolean, runHooks: boolean): string { + return `${force ? 'force' : 'normal'}:${runHooks ? 'run-hooks' : 'skip-hooks'}` } function getRuntimeFolderWorkspaceRootId(repo: Repo): string { @@ -15534,16 +15527,14 @@ export class OrcaRuntimeService { async removeManagedWorktree( worktreeSelector: string, force = false, - runHooks = false, - overrideLock = false + runHooks = false ): Promise { if (!this.store) { throw new Error('runtime_unavailable') } - assertWorktreeRemovalForcePermissions(force, overrideLock) const store = this.store const removalTarget = await this.resolveWorktreeRemovalTarget(worktreeSelector) - const optionsKey = getRuntimeWorktreeRemovalOptionsKey(force, runHooks, overrideLock) + const optionsKey = getRuntimeWorktreeRemovalOptionsKey(force, runHooks) const inFlightRemoval = this.removeManagedWorktreeInFlight.get(removalTarget.id) if (inFlightRemoval) { if (inFlightRemoval.optionsKey === optionsKey) { @@ -15746,7 +15737,7 @@ export class OrcaRuntimeService { // Why: a Git lock must block before archive hooks or linked-path cleanup // mutate the workspace; dirty-file force is a separate permission. try { - assertWorktreeUnlockedForRemoval(registeredWorktree, overrideLock) + assertWorktreeUnlockedForRemoval(registeredWorktree) } catch (error) { throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force)) } @@ -15766,8 +15757,7 @@ export class OrcaRuntimeService { repoPath: repo.path, localWorktreeGitOptions, registeredWorktree, - deleteBranch, - overrideLock + deleteBranch }) await cleanupUnusedWorktreePushTargetRemote( repo.path, @@ -15790,10 +15780,7 @@ export class OrcaRuntimeService { return removalResult ?? {} } if (repo.connectionId) { - const remoteRemoveOptions = { - ...(!deleteBranch ? { deleteBranch } : {}), - ...(overrideLock ? { overrideLock: true } : {}) - } + const remoteRemoveOptions = !deleteBranch ? { deleteBranch } : {} const rawRemovalResult = await (Object.keys(remoteRemoveOptions).length > 0 ? provider!.removeWorktree(canonicalWorktreePath, force, remoteRemoveOptions) : provider!.removeWorktree(canonicalWorktreePath, force)) @@ -15857,7 +15844,7 @@ export class OrcaRuntimeService { try { // Why: an archive hook can race another Git client that locks the row; // recheck before linked-path, watcher, or terminal teardown side effects. - assertWorktreeUnlockedForRemoval(refreshedRegisteredWorktree, overrideLock) + assertWorktreeUnlockedForRemoval(refreshedRegisteredWorktree) } catch (error) { throw new Error(formatWorktreeRemovalError(error, canonicalWorktreePath, force)) } @@ -15914,7 +15901,6 @@ export class OrcaRuntimeService { try { const removeOptions = { ...(!deleteBranch ? { deleteBranch } : {}), - ...(overrideLock ? { overrideLock: true } : {}), // Why: removal already validated the Git row under the selected // project runtime; keep branch cleanup on that same canonical row. knownRemovedWorktree: refreshedRegisteredWorktree, @@ -15935,7 +15921,6 @@ export class OrcaRuntimeService { localWorktreeGitOptions, registeredWorktree: refreshedRegisteredWorktree, deleteBranch, - overrideLock, closeWatcher: (worktreePath) => closeLocalWatcherForWorktreePath(worktreePath).catch((err) => { console.warn(`[filesystem-watcher] failed to close ${worktreePath}:`, err) diff --git a/src/main/runtime/rpc/methods/worktree-schemas.ts b/src/main/runtime/rpc/methods/worktree-schemas.ts index 8d05ccdc5a7..96f2813576e 100644 --- a/src/main/runtime/rpc/methods/worktree-schemas.ts +++ b/src/main/runtime/rpc/methods/worktree-schemas.ts @@ -224,7 +224,6 @@ export const WorktreeSet = WorktreeSelector.extend({ export const WorktreeRemove = WorktreeSelector.extend({ force: OptionalBoolean, - overrideLock: OptionalBoolean, runHooks: OptionalBoolean }) diff --git a/src/main/runtime/rpc/methods/worktree.test.ts b/src/main/runtime/rpc/methods/worktree.test.ts index ccdde78ee3c..ef7ef3910f0 100644 --- a/src/main/runtime/rpc/methods/worktree.test.ts +++ b/src/main/runtime/rpc/methods/worktree.test.ts @@ -67,7 +67,7 @@ describe('worktree RPC methods', () => { }) }) - it('routes an explicit lock override separately from dirty-file force', async () => { + it('routes dirty-file force to the runtime server', async () => { const runtime = { getRuntimeId: () => 'test-runtime', removeManagedWorktree: vi.fn().mockResolvedValue({}) @@ -78,12 +78,11 @@ describe('worktree RPC methods', () => { makeRequest('worktree.rm', { worktree: 'id:wt-1', force: true, - overrideLock: true, runHooks: false }) ) - expect(runtime.removeManagedWorktree).toHaveBeenCalledWith('id:wt-1', true, false, true) + expect(runtime.removeManagedWorktree).toHaveBeenCalledWith('id:wt-1', true, false) expect(response).toMatchObject({ ok: true, result: { removed: true } }) }) diff --git a/src/main/runtime/rpc/methods/worktree.ts b/src/main/runtime/rpc/methods/worktree.ts index 0d002b1596b..f84a26d1d87 100644 --- a/src/main/runtime/rpc/methods/worktree.ts +++ b/src/main/runtime/rpc/methods/worktree.ts @@ -230,8 +230,7 @@ export const WORKTREE_METHODS: RpcMethod[] = [ const result = await runtime.removeManagedWorktree( params.worktree, params.force === true, - params.runHooks === true, - params.overrideLock === true + params.runHooks === true ) return { removed: true, ...result } } diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 3297d2e5183..3496e31a029 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1122,7 +1122,6 @@ export type PreloadApi = { remove: (args: { worktreeId: string force?: boolean - overrideLock?: boolean skipArchive?: boolean }) => Promise // Forget a workspace from Orca only — no remote Git/filesystem work. Used diff --git a/src/relay/git-handler-worktree-ops.test.ts b/src/relay/git-handler-worktree-ops.test.ts index d8ea6d51811..e16d4d61fc9 100644 --- a/src/relay/git-handler-worktree-ops.test.ts +++ b/src/relay/git-handler-worktree-ops.test.ts @@ -258,7 +258,7 @@ describe('removeWorktreeOp', () => { expect(git).not.toHaveBeenCalledWith(['branch', '-d', '--', 'feature/test'], expect.any(String)) }) - it('uses double force only when the SSH request explicitly overrides a lock', async () => { + it('does not let force override a locked SSH worktree', async () => { const git = vi.fn(async (args) => { if (args[0] === 'rev-parse') { return { stdout: '/repo/.git\n', stderr: '' } @@ -275,28 +275,16 @@ describe('removeWorktreeOp', () => { return { stdout: '', stderr: '' } }) - await removeWorktreeOp(git, { - worktreePath: '/repo-feature', - force: true, - overrideLock: true - }) + await expect( + removeWorktreeOp(git, { worktreePath: '/repo-feature', force: true }) + ).rejects.toThrow('Worktree is locked by Git. Lock reason: remote agent') - expect(git).toHaveBeenCalledWith( - ['worktree', 'remove', '--force', '--force', '/repo-feature'], + expect(git).not.toHaveBeenCalledWith( + ['worktree', 'remove', '--force', '/repo-feature'], expect.any(String) ) }) - it('rejects SSH lock override without dirty-file force before invoking Git', async () => { - const git = vi.fn() - - await expect( - removeWorktreeOp(git, { worktreePath: '/repo-feature', overrideLock: true }) - ).rejects.toThrow('Worktree lock override requires force deletion permission.') - - expect(git).not.toHaveBeenCalled() - }) - it('skips branch deletion entirely when deleteBranch is false', async () => { const calls: string[] = [] const git = vi.fn(async (args, cwd) => { diff --git a/src/relay/git-handler-worktree-remove.ts b/src/relay/git-handler-worktree-remove.ts index fcbf2608168..7c426facc1e 100644 --- a/src/relay/git-handler-worktree-remove.ts +++ b/src/relay/git-handler-worktree-remove.ts @@ -1,9 +1,6 @@ import * as path from 'node:path' import type { RemoveWorktreeResult } from '../shared/types' -import { - assertWorktreeRemovalForcePermissions, - assertWorktreeUnlockedForRemoval -} from '../shared/worktree-removal' +import { assertWorktreeUnlockedForRemoval } from '../shared/worktree-removal' import { deleteAlreadyMergedRelayBranchAfterSafeDeleteFailure } from './git-handler-branch-cleanup' import type { GitExec } from './git-handler-ops' import { isUnsupportedWorktreeListZError, parseWorktreeList } from './git-handler-utils' @@ -160,10 +157,8 @@ export async function removeWorktreeOp( ): Promise { const worktreePath = params.worktreePath as string const force = params.force as boolean | undefined - const overrideLock = params.overrideLock === true const deleteBranch = params.deleteBranch !== false const forceBranchDelete = params.forceBranchDelete === true - assertWorktreeRemovalForcePermissions(force === true, overrideLock) let repoPath = worktreePath try { @@ -183,14 +178,10 @@ export async function removeWorktreeOp( const branchName = normalizeLocalBranchRef(removedWorktree?.branch ?? '') const branchHead = removedWorktree?.head ?? '' - assertWorktreeUnlockedForRemoval(removedWorktree, overrideLock) + assertWorktreeUnlockedForRemoval(removedWorktree) const args = ['worktree', 'remove'] - if (overrideLock) { - // Why: mirror local deletion semantics; a second force is reserved for - // the explicit lock-recovery action, not ordinary dirty-file deletion. - args.push('--force', '--force') - } else if (force) { + if (force) { args.push('--force') } args.push(worktreePath) diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx index 5d411e08b8f..7f1576c720b 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.test.tsx @@ -25,7 +25,7 @@ const mocks = vi.hoisted(() => { isDeleting: boolean error: string | null canForceDelete: boolean - forceDeleteReason: 'dirty' | 'locked' | 'orphan-directory' | 'missing-registration' | null + forceDeleteReason: 'dirty' | 'orphan-directory' | 'missing-registration' | null lockReason?: string | null } > @@ -262,7 +262,7 @@ describe('DeleteWorktreeDialog lineage copy', () => { expect(markup).not.toContain('Also delete local branch') }) - it('shows locked and known dirty details and overrides the lock only on recovery', async () => { + it('shows locked and known dirty details without offering a lock override', async () => { const workspace = makeWorktree('Locked workspace', '/workspaces/locked') mocks.state.modalData = { worktreeId: workspace.id } mocks.state.allWorktrees.mockReturnValue([workspace]) @@ -273,8 +273,8 @@ describe('DeleteWorktreeDialog lineage copy', () => { [workspace.id]: { isDeleting: false, error: 'Worktree is locked by Git. Lock reason: active agent session.', - canForceDelete: true, - forceDeleteReason: 'locked', + canForceDelete: false, + forceDeleteReason: null, lockReason: 'active agent session' } } @@ -283,18 +283,9 @@ describe('DeleteWorktreeDialog lineage copy', () => { const markup = renderToStaticMarkup() expect(markup).toContain('Worktree is locked by Git. Lock reason: active agent session.') - expect(markup).toContain('Force Delete') + expect(markup).not.toContain('Force Delete') expect(markup).toContain('1 uncommitted or untracked change') - - mocks.state.removeWorktree.mockResolvedValue({ ok: true }) - const forceButton = mocks.buttonProps.find((props) => - buttonText(props).includes('Force Delete') - ) - ;(forceButton as { onClick?: () => void } | undefined)?.onClick?.() - - expect(mocks.state.removeWorktree).toHaveBeenCalledWith(workspace.id, true, { - overrideLock: true - }) + expect(mocks.state.removeWorktree).not.toHaveBeenCalled() }) it('notifies the dialog caller after a toast force delete succeeds', async () => { diff --git a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx index ae52b8b0931..598f5bedd65 100644 --- a/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx +++ b/src/renderer/src/components/sidebar/DeleteWorktreeDialog.tsx @@ -266,10 +266,7 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { // the shared toast wrapper. Close immediately because workspace cards // already show the deleting state while the retry runs. const commitFocus = prepareActiveWorktreeFocusAfterDelete(worktreeId) - const deletePromise = - deleteState?.forceDeleteReason === 'locked' - ? removeWorktree(worktreeId, true, { overrideLock: true }) - : removeWorktree(worktreeId, true) + const deletePromise = removeWorktree(worktreeId, true) closeModal() deletePromise .then((result) => { @@ -325,7 +322,6 @@ const DeleteWorktreeDialog = React.memo(function DeleteWorktreeDialog() { onDeleted, persistDontAskAgainPreference, removeWorktree, - deleteState?.forceDeleteReason, worktreeIds.length, worktreeId, worktrees diff --git a/src/renderer/src/components/sidebar/delete-worktree-failure-toast.test.tsx b/src/renderer/src/components/sidebar/delete-worktree-failure-toast.test.tsx index 4c0ae793bda..9c5dc9fbce3 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-failure-toast.test.tsx +++ b/src/renderer/src/components/sidebar/delete-worktree-failure-toast.test.tsx @@ -119,13 +119,13 @@ describe('showDeleteWorktreeFailureToast', () => { expect(onViewChanges).toHaveBeenCalled() }) - it('offers force delete without View for a locked workspace', () => { + it('offers neither force delete nor View for a locked workspace', () => { const onViewChanges = vi.fn() showDeleteWorktreeFailureToast({ error: 'Worktree is locked by Git.', - canForceDelete: true, - forceDeleteReason: 'locked', + canForceDelete: false, + forceDeleteReason: null, onViewChanges, onForceDelete: vi.fn(), worktreeId: 'wt-locked', @@ -134,7 +134,8 @@ describe('showDeleteWorktreeFailureToast', () => { const body = renderToastBody('info') expect(body.textContent).toContain('This workspace is locked by Git.') - expect(body.textContent).toContain('Force Delete') + expect(body.textContent).toContain('Unlock it manually with git worktree unlock') + expect(body.textContent).not.toContain('Force Delete') expect(body.textContent).not.toContain('View') expect(onViewChanges).not.toHaveBeenCalled() }) @@ -142,8 +143,8 @@ describe('showDeleteWorktreeFailureToast', () => { it('keeps View for a locked workspace when changed files are known', () => { showDeleteWorktreeFailureToast({ error: 'Worktree is locked by Git.', - canForceDelete: true, - forceDeleteReason: 'locked', + canForceDelete: false, + forceDeleteReason: null, lockReason: 'active agent session', hasKnownChanges: true, onViewChanges: vi.fn(), diff --git a/src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx b/src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx index d463b136340..bb9649f28f3 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx +++ b/src/renderer/src/components/sidebar/delete-worktree-failure-toast.tsx @@ -2,7 +2,10 @@ import { toast } from 'sonner' import { Button } from '../ui/button' import { getDeleteWorktreeToastCopy } from './delete-worktree-toast' import { translate } from '@/i18n/i18n' -import type { WorktreeForceDeleteReason } from '../../../../shared/worktree-removal' +import { + isLockedWorktreeRemovalError, + type WorktreeForceDeleteReason +} from '../../../../shared/worktree-removal' type DeleteWorktreeFailureToastOptions = { error: string @@ -93,7 +96,7 @@ export function showDeleteWorktreeFailureToast({ { isDeleting?: boolean error?: string | null canForceDelete?: boolean - forceDeleteReason?: 'dirty' | 'locked' | null + forceDeleteReason?: 'dirty' | null lockReason?: string | null } > @@ -203,15 +203,15 @@ describe('runWorktreeBatchDelete', () => { }) }) - it('sets overrideLock only for the explicit locked-worktree recovery action', async () => { + it('does not offer force delete for a locked worktree', async () => { mocks.state.settings = { skipDeleteWorktreeConfirm: true } mocks.state.removeWorktree .mockImplementationOnce(async (worktreeId: string) => { mocks.state.deleteStateByWorktreeId[worktreeId] = { isDeleting: false, error: 'Worktree is locked by Git.', - canForceDelete: true, - forceDeleteReason: 'locked', + canForceDelete: false, + forceDeleteReason: null, lockReason: 'active agent session' } return { ok: false, error: 'Worktree is locked by Git.' } @@ -224,16 +224,11 @@ describe('runWorktreeBatchDelete', () => { await vi.waitFor(() => expect(showDeleteWorktreeFailureToast).toHaveBeenCalled()) const toastOptions = vi.mocked(showDeleteWorktreeFailureToast).mock.calls[0]?.[0] expect(toastOptions).toMatchObject({ - forceDeleteReason: 'locked', + canForceDelete: false, + forceDeleteReason: null, lockReason: 'active agent session' }) - toastOptions?.onForceDelete() - - await vi.waitFor(() => { - expect(mocks.state.removeWorktree).toHaveBeenNthCalledWith(2, 'wt-1', true, { - overrideLock: true - }) - }) + expect(mocks.state.removeWorktree).toHaveBeenCalledTimes(1) }) it('keeps parent worktree deletes behind confirmation even when confirmation is skipped', () => { diff --git a/src/renderer/src/components/sidebar/delete-worktree-flow.ts b/src/renderer/src/components/sidebar/delete-worktree-flow.ts index eb83776d6c6..ee1f9f70d18 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-flow.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-flow.ts @@ -161,10 +161,7 @@ export function runWorktreeDeleteWithToast( // while the failed-delete toast was open, so focus only hands off // when this is still the workspace they are viewing. const commitForceFocus = prepareActiveWorktreeFocusAfterDelete(worktreeId) - const forceRemoval = - state?.forceDeleteReason === 'locked' - ? useAppStore.getState().removeWorktree(worktreeId, true, { overrideLock: true }) - : useAppStore.getState().removeWorktree(worktreeId, true) + const forceRemoval = useAppStore.getState().removeWorktree(worktreeId, true) forceRemoval .then((forceResult) => { if (!forceResult.ok) { 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 5859f2f21b9..1d21ca6e049 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-toast.test.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-toast.test.ts @@ -48,20 +48,26 @@ describe('getDeleteWorktreeToastCopy', () => { }) it('uses lock-specific guidance', () => { - expect(getDeleteWorktreeToastCopy('feature/foo', 'locked', 'raw lock error')).toEqual({ + expect(getDeleteWorktreeToastCopy('feature/foo', null, 'Worktree is locked by Git.')).toEqual({ title: 'Failed to delete workspace feature/foo', - description: 'This workspace is locked by Git. Use Force Delete to remove it anyway.', + description: + 'This workspace is locked by Git. Unlock it manually with git worktree unlock before deleting it.', isDestructive: false }) }) it('includes the structured Git lock reason in localized recovery copy', () => { expect( - getDeleteWorktreeToastCopy('feature/foo', 'locked', 'raw lock error', 'active agent session') + getDeleteWorktreeToastCopy( + 'feature/foo', + null, + 'Worktree is locked by Git. Lock reason: active agent session.', + 'active agent session' + ) ).toEqual({ title: 'Failed to delete workspace feature/foo', description: - 'This workspace is locked by Git. Git reported: active agent session. Use Force Delete to remove it anyway.', + 'This workspace is locked by Git. Git reported: active agent session. Unlock it manually with git worktree unlock before deleting it.', isDestructive: false }) }) diff --git a/src/renderer/src/components/sidebar/delete-worktree-toast.ts b/src/renderer/src/components/sidebar/delete-worktree-toast.ts index cb85133bc7e..e84d435f8cc 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-toast.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-toast.ts @@ -1,5 +1,8 @@ import { translate } from '@/i18n/i18n' -import type { WorktreeForceDeleteReason } from '../../../../shared/worktree-removal' +import { + isLockedWorktreeRemovalError, + type WorktreeForceDeleteReason +} from '../../../../shared/worktree-removal' export type DeleteWorktreeToastCopy = { title: string description?: string @@ -12,27 +15,28 @@ export function getDeleteWorktreeToastCopy( error: string, lockReason: string | null = null ): DeleteWorktreeToastCopy { - if (forceDeleteReason) { - if (forceDeleteReason === 'locked') { - return { - title: translate( - 'auto.components.sidebar.delete.worktree.toast.1d0fa5c0a5', - 'Failed to delete workspace {{value0}}', - { value0: worktreeName } - ), - description: lockReason - ? translate( - 'auto.components.sidebar.delete.worktree.toast.lockedReason', - 'This workspace is locked by Git. Git reported: {{value0}}. Use Force Delete to remove it anyway.', - { value0: lockReason } - ) - : translate( - 'auto.components.sidebar.delete.worktree.toast.locked', - 'This workspace is locked by Git. Use Force Delete to remove it anyway.' - ), - isDestructive: false - } + if (isLockedWorktreeRemovalError(error)) { + return { + title: translate( + 'auto.components.sidebar.delete.worktree.toast.1d0fa5c0a5', + 'Failed to delete workspace {{value0}}', + { value0: worktreeName } + ), + description: lockReason + ? translate( + 'auto.components.sidebar.delete.worktree.toast.lockedReason', + 'This workspace is locked by Git. Git reported: {{value0}}. Unlock it manually with git worktree unlock before deleting it.', + { value0: lockReason } + ) + : translate( + 'auto.components.sidebar.delete.worktree.toast.locked', + 'This workspace is locked by Git. Unlock it manually with git worktree unlock before deleting it.' + ), + isDestructive: false } + } + + if (forceDeleteReason) { if (forceDeleteReason === 'orphan-directory') { return { title: translate( diff --git a/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx b/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx index 675f4ec9fe7..734a531fc01 100644 --- a/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx +++ b/src/renderer/src/components/status-bar/WorkspaceSpaceManagerPanel.tsx @@ -83,7 +83,6 @@ import { type WorkspaceSpaceSortKey } from './workspace-space-presentation' import { translate } from '@/i18n/i18n' -import { forceDeleteWorkspaceFromSpace } from './workspace-space-force-delete' import type { WorktreeForceDeleteReason } from '../../../../shared/worktree-removal' const TREEMAP_FILLS = [ @@ -1601,9 +1600,7 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { // Why: Space keeps normal deletes non-force so uncommitted work is not // discarded silently; a failed row gets this explicit recovery path. const commitFocus = prepareActiveWorktreeFocusAfterDelete(worktree.worktreeId) - const forceDeleteReason = - deleteStateByWorktreeId[worktree.worktreeId]?.forceDeleteReason ?? null - void forceDeleteWorkspaceFromSpace(removeWorktree, worktree.worktreeId, forceDeleteReason) + void removeWorktree(worktree.worktreeId, true) .then((result) => { if (!result.ok) { toast.error( @@ -1632,7 +1629,7 @@ export function WorkspaceSpaceManagerPanel(): React.JSX.Element { ) }) }, - [deleteStateByWorktreeId, handleDeletedWorktrees, removeWorktree] + [handleDeletedWorktrees, removeWorktree] ) const deleteSelected = (): void => { diff --git a/src/renderer/src/components/status-bar/workspace-space-force-delete.test.ts b/src/renderer/src/components/status-bar/workspace-space-force-delete.test.ts deleted file mode 100644 index aa31daf0848..00000000000 --- a/src/renderer/src/components/status-bar/workspace-space-force-delete.test.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { describe, expect, it, vi } from 'vitest' -import { forceDeleteWorkspaceFromSpace } from './workspace-space-force-delete' - -describe('forceDeleteWorkspaceFromSpace', () => { - it('passes lock override only for an explicitly locked row', async () => { - const removeWorktree = vi.fn().mockResolvedValue({ ok: true }) - - await forceDeleteWorkspaceFromSpace(removeWorktree, 'locked-worktree', 'locked') - await forceDeleteWorkspaceFromSpace(removeWorktree, 'dirty-worktree', 'dirty') - - expect(removeWorktree).toHaveBeenNthCalledWith(1, 'locked-worktree', true, { - overrideLock: true - }) - expect(removeWorktree).toHaveBeenNthCalledWith(2, 'dirty-worktree', true, undefined) - }) - - it.each(['orphan-directory', 'missing-registration'] as const)( - 'keeps %s recovery on ordinary force semantics', - async (reason) => { - const removeWorktree = vi.fn().mockResolvedValue({ ok: true }) - - await forceDeleteWorkspaceFromSpace(removeWorktree, 'worktree', reason) - - expect(removeWorktree).toHaveBeenCalledWith('worktree', true, undefined) - } - ) -}) diff --git a/src/renderer/src/components/status-bar/workspace-space-force-delete.ts b/src/renderer/src/components/status-bar/workspace-space-force-delete.ts deleted file mode 100644 index 7dc57737429..00000000000 --- a/src/renderer/src/components/status-bar/workspace-space-force-delete.ts +++ /dev/null @@ -1,17 +0,0 @@ -import type { WorktreeForceDeleteReason } from '../../../../shared/worktree-removal' - -type RemoveWorktreeFromSpace = ( - worktreeId: string, - force?: boolean, - options?: { overrideLock?: boolean } -) => Promise - -export function forceDeleteWorkspaceFromSpace( - removeWorktree: RemoveWorktreeFromSpace, - worktreeId: string, - reason: WorktreeForceDeleteReason | null -): Promise { - // Why: dirty and orphan recovery need ordinary force; only a confirmed Git - // lock may opt into Git's separate double-force override. - return removeWorktree(worktreeId, true, reason === 'locked' ? { overrideLock: true } : undefined) -} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 55770283afd..0641f95e3e5 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -4305,8 +4305,8 @@ "ead7b8ee15": "It has changed files. Use Force Delete to delete it anyway.", "905fc8efac": "Git already removed this workspace. Use Force Delete to clear it from Orca.", "0899ebdb28": "Git already forgot this workspace, but its directory is still on disk. Use Force Delete to remove the orphaned directory.", - "locked": "This workspace is locked by Git. Use Force Delete to remove it anyway.", - "lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Use Force Delete to remove it anyway." + "locked": "This workspace is locked by Git. Unlock it manually with git worktree unlock before deleting it.", + "lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Unlock it manually with git worktree unlock before deleting it." } } }, diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 362ef7f8efb..61d64fd32ff 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -4282,8 +4282,8 @@ "ead7b8ee15": "Tiene archivos modificados. Usa Forzar eliminación para eliminarlo de todos modos.", "905fc8efac": "Git ya eliminó este espacio de trabajo. Usa Forzar eliminación para borrarlo de Orca.", "0899ebdb28": "Git ya olvidó este espacio de trabajo, pero su directorio todavía está en disco. Usa Forzar eliminación para eliminar el directorio huérfano.", - "locked": "This workspace is locked by Git. Use Force Delete to remove it anyway.", - "lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Use Force Delete to remove it anyway." + "locked": "This workspace is locked by Git. Unlock it manually with git worktree unlock before deleting it.", + "lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Unlock it manually with git worktree unlock before deleting it." } } }, diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 7de82090315..0d719a8dad5 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -4263,8 +4263,8 @@ "ead7b8ee15": "ファイルが変更されました。とにかく削除するには、強制削除を使用してください。", "905fc8efac": "Git はこのワークスペースをすでに削除しています。 Orca から削除するには、強制削除を使用します。", "0899ebdb28": "Git はすでにこのワークスペースを忘れていますが、そのディレクトリはまだディスク上にあります。孤立したディレクトリを削除するには、強制削除を使用します。", - "locked": "This workspace is locked by Git. Use Force Delete to remove it anyway.", - "lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Use Force Delete to remove it anyway." + "locked": "This workspace is locked by Git. Unlock it manually with git worktree unlock before deleting it.", + "lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Unlock it manually with git worktree unlock before deleting it." } } }, diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 85e11e97435..0560c699e94 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -4263,8 +4263,8 @@ "ead7b8ee15": "파일이 변경되었습니다. 어쨌든 삭제하려면 강제 삭제를 사용하세요.", "905fc8efac": "Git은 이미 이 워크스페이스를 제거했습니다. 강제 삭제를 사용하여 Orca에서 삭제합니다.", "0899ebdb28": "Git은 이미 이 워크스페이스를 잊어버렸지만 해당 디렉토리는 여전히 디스크에 있습니다. 강제 삭제를 사용하여 분리된 디렉터리를 제거합니다.", - "locked": "This workspace is locked by Git. Use Force Delete to remove it anyway.", - "lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Use Force Delete to remove it anyway." + "locked": "This workspace is locked by Git. Unlock it manually with git worktree unlock before deleting it.", + "lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Unlock it manually with git worktree unlock before deleting it." } } }, diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 1f00e4731ad..e4dd6de47ae 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -4263,8 +4263,8 @@ "ead7b8ee15": "它已更改文件。无论如何,请使用强制删除将其删除。", "905fc8efac": "Git 已经删除了这个工作区。使用强制删除将其从 Orca 中清除。", "0899ebdb28": "Git 已经忘记了这个工作区,但它的目录仍然在磁盘上。使用强制删除来删除孤立目录。", - "locked": "This workspace is locked by Git. Use Force Delete to remove it anyway.", - "lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Use Force Delete to remove it anyway." + "locked": "This workspace is locked by Git. Unlock it manually with git worktree unlock before deleting it.", + "lockedReason": "This workspace is locked by Git. Git reported: {{value0}}. Unlock it manually with git worktree unlock before deleting it." } } }, diff --git a/src/renderer/src/store/slices/store-cascades.test.ts b/src/renderer/src/store/slices/store-cascades.test.ts index 2bb9463daff..ae2266156b3 100644 --- a/src/renderer/src/store/slices/store-cascades.test.ts +++ b/src/renderer/src/store/slices/store-cascades.test.ts @@ -440,8 +440,8 @@ describe('removeWorktree cascade', () => { expect(store.getState().deleteStateByWorktreeId[worktreeId]).toEqual({ isDeleting: false, error, - canForceDelete: true, - forceDeleteReason: 'locked', + canForceDelete: false, + forceDeleteReason: null, lockReason: null }) }) diff --git a/src/renderer/src/store/slices/worktree-helpers.ts b/src/renderer/src/store/slices/worktree-helpers.ts index 40cb2af630d..cd126bc7a70 100644 --- a/src/renderer/src/store/slices/worktree-helpers.ts +++ b/src/renderer/src/store/slices/worktree-helpers.ts @@ -187,7 +187,6 @@ export type WorktreeSlice = { // renderer-side teardown/purge as a normal remove. options?: { mode?: 'remove' | 'forget-local' - overrideLock?: boolean suppressPreservedBranchToast?: boolean } ) => Promise<({ ok: true } & RemoveWorktreeResult) | { ok: false; error: string }> diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index a21fd9f91b9..16117349225 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -68,7 +68,8 @@ import { import { folderWorkspaceToWorktree } from '../../../../shared/folder-workspace-worktree' import { classifyWorktreeForceDeleteReason, - getLockedWorktreeRemovalReason + getLockedWorktreeRemovalReason, + isLockedWorktreeRemovalError } from '../../../../shared/worktree-removal' export type { WorktreeSlice, WorktreeDeleteState } from './worktree-helpers' @@ -3160,7 +3161,6 @@ export const createWorktreeSlice: StateCreator ? window.api.worktrees.remove({ worktreeId, force, - overrideLock: options?.overrideLock, skipArchive }) : callRuntimeRpc( @@ -3169,7 +3169,6 @@ export const createWorktreeSlice: StateCreator { worktree: toRuntimeWorktreeSelector(worktreeId), force, - overrideLock: options?.overrideLock, runHooks: !skipArchive }, { timeoutMs: 60_000 } @@ -3512,11 +3511,8 @@ export const createWorktreeSlice: StateCreator // handled user decision point surfaced by the delete toast, not an app error. console.warn('Failed to remove worktree:', err) const error = err instanceof Error ? err.message : String(err) - const forceDeleteReason = classifyWorktreeForceDeleteReason( - error, - force, - options?.overrideLock === true - ) + const forceDeleteReason = classifyWorktreeForceDeleteReason(error, force) + const locked = isLockedWorktreeRemovalError(error) set((s) => ({ deleteStateByWorktreeId: { ...s.deleteStateByWorktreeId, @@ -3525,9 +3521,7 @@ export const createWorktreeSlice: StateCreator error, canForceDelete: forceDeleteReason !== null, forceDeleteReason, - ...(forceDeleteReason === 'locked' - ? { lockReason: getLockedWorktreeRemovalReason(error) } - : {}) + ...(locked ? { lockReason: getLockedWorktreeRemovalReason(error) } : {}) } } })) diff --git a/src/renderer/src/web/web-preload-api.test.ts b/src/renderer/src/web/web-preload-api.test.ts index 21057bfd742..743b5d786da 100644 --- a/src/renderer/src/web/web-preload-api.test.ts +++ b/src/renderer/src/web/web-preload-api.test.ts @@ -1725,7 +1725,6 @@ describe('web worktree preload API', () => { await globals.window.api.worktrees.remove({ worktreeId: 'repo-1::/workspace/locked', force: true, - overrideLock: true, skipArchive: false }) await globals.window.api.worktrees.remove({ @@ -1740,7 +1739,6 @@ describe('web worktree preload API', () => { params: { worktree: 'id:repo-1::/workspace/locked', force: true, - overrideLock: true, runHooks: true } }, @@ -1749,7 +1747,6 @@ describe('web worktree preload API', () => { params: { worktree: 'id:repo-1::/workspace/dirty', force: true, - overrideLock: undefined, runHooks: false } } diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 920a47dd53f..0e144736b5f 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -1396,12 +1396,11 @@ function createWorktreesApi(): NonNullable['worktrees']> { targetBranch, isCrossRepository }), - remove: async ({ worktreeId, force, overrideLock, skipArchive }) => { + remove: async ({ worktreeId, force, skipArchive }) => { invalidateRuntimeWorktreeCaches() return callRuntimeResult('worktree.rm', { worktree: toRuntimeWorktreeSelector(worktreeId), force, - overrideLock, runHooks: skipArchive !== true }) }, diff --git a/src/shared/worktree-removal.ts b/src/shared/worktree-removal.ts index 5505ff9b7b8..bc71f121bf2 100644 --- a/src/shared/worktree-removal.ts +++ b/src/shared/worktree-removal.ts @@ -1,39 +1,31 @@ import type { GitWorktreeInfo } from './types' export const LOCKED_WORKTREE_REMOVAL_PREFIX = 'Worktree is locked by Git.' -export const LOCK_OVERRIDE_REQUIRES_FORCE_MESSAGE = - 'Worktree lock override requires force deletion permission.' -export type WorktreeForceDeleteReason = - | 'dirty' - | 'locked' - | 'orphan-directory' - | 'missing-registration' +export type WorktreeForceDeleteReason = 'dirty' | 'orphan-directory' | 'missing-registration' export function createLockedWorktreeRemovalError(lockReason?: string): Error { const reason = lockReason?.trim() return new Error( reason - ? `${LOCKED_WORKTREE_REMOVAL_PREFIX} Lock reason: ${reason}. Use Force Delete to remove it anyway.` - : `${LOCKED_WORKTREE_REMOVAL_PREFIX} Use Force Delete to remove it anyway.` + ? `${LOCKED_WORKTREE_REMOVAL_PREFIX} Lock reason: ${reason}. Unlock it manually with git worktree unlock before deleting it.` + : `${LOCKED_WORKTREE_REMOVAL_PREFIX} Unlock it manually with git worktree unlock before deleting it.` ) } export function assertWorktreeUnlockedForRemoval( - worktree: Pick | undefined, - overrideLock = false + worktree: Pick | undefined ): void { - if (!overrideLock && worktree?.locked) { + if (worktree?.locked) { throw createLockedWorktreeRemovalError(worktree.lockReason) } } -export function assertWorktreeRemovalForcePermissions(force = false, overrideLock = false): void { - if (overrideLock && !force) { - // Why: Git's second force also discards dirty files, so lock override must - // never grant a permission the caller did not explicitly provide. - throw new Error(LOCK_OVERRIDE_REQUIRES_FORCE_MESSAGE) - } +export function isLockedWorktreeRemovalError(error: string): boolean { + return ( + error.includes(LOCKED_WORKTREE_REMOVAL_PREFIX) || + error.includes('cannot remove a locked working tree') + ) } export function getLockedWorktreeRemovalReason(error: string): string | null { @@ -42,7 +34,7 @@ export function getLockedWorktreeRemovalReason(error: string): string | null { return null } const reasonStart = prefixIndex + `${LOCKED_WORKTREE_REMOVAL_PREFIX} Lock reason: `.length - const recoverySuffix = '. Use Force Delete to remove it anyway.' + const recoverySuffix = '. Unlock it manually with git worktree unlock before deleting it.' const suffixIndex = error.indexOf(recoverySuffix, reasonStart) const reason = error.slice(reasonStart, suffixIndex === -1 ? undefined : suffixIndex).trim() return reason || null @@ -53,20 +45,13 @@ const FORMATTED_DIRTY_WORKTREE_REMOVAL_PATTERN = export function classifyWorktreeForceDeleteReason( error: string, - force = false, - overrideLock = false + force = false ): WorktreeForceDeleteReason | null { - if (overrideLock) { + if (isLockedWorktreeRemovalError(error)) { + // Why: a Git lock can represent an external safety contract. It must be + // unlocked explicitly rather than folded into Orca's dirty-file force path. return null } - if ( - error.includes(LOCKED_WORKTREE_REMOVAL_PREFIX) || - error.includes('cannot remove a locked working tree') - ) { - // Why: raw Git matching keeps force recovery available with older relays; - // current runtimes emit the locale-independent app-owned prefix above. - return 'locked' - } if (force) { return null }