diff --git a/src/main/git/remove-worktree.test.ts b/src/main/git/remove-worktree.test.ts index 74f052b3160..991cd382d75 100644 --- a/src/main/git/remove-worktree.test.ts +++ b/src/main/git/remove-worktree.test.ts @@ -272,6 +272,14 @@ 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': { diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index fc5d61b0fd1..e6194b05109 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -13,7 +13,10 @@ import type { LocalBaseRefUpdateSuggestion, RemoveWorktreeResult } from '../../shared/types' -import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree-removal' +import { + assertWorktreeRemovalForcePermissions, + 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' @@ -1077,6 +1080,7 @@ async function performRemoveWorktree( force = false, options: RemoveWorktreeOptions = {} ): Promise { + assertWorktreeRemovalForcePermissions(force, options.overrideLock) const removedWorktree = options.knownRemovedWorktree ?? (await listWorktrees(repoPath, options)).find((worktree) => diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 2f75e72a196..e6447cca047 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -7798,6 +7798,19 @@ 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 469a06a615f..8735948a59e 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -29,7 +29,10 @@ import type { Worktree, WorktreeMeta } from '../../shared/types' -import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree-removal' +import { + assertWorktreeRemovalForcePermissions, + assertWorktreeUnlockedForRemoval +} from '../../shared/worktree-removal' import { buildKnownOrcaWorkspaceLayouts, isLegacyRepoForExternalWorktreeVisibility, @@ -1360,6 +1363,7 @@ export function registerWorktreeHandlers( skipArchive?: boolean } ) => { + assertWorktreeRemovalForcePermissions(args.force === true, args.overrideLock === true) const optionsKey = getWorktreeRemovalOptionsKey(args) const inFlightRemoval = worktreeRemovalsInFlight.get(args.worktreeId) if (inFlightRemoval) { diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index abdb5e8b3e3..e8e4a3d0a95 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -26762,6 +26762,21 @@ 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 44c0e707dc8..36f8d047c23 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -127,7 +127,10 @@ import type { WorkspaceSessionState, DirEntry } from '../../shared/types' -import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree-removal' +import { + assertWorktreeRemovalForcePermissions, + assertWorktreeUnlockedForRemoval +} from '../../shared/worktree-removal' import { getRepoExecutionHostId, parseExecutionHostId, @@ -15537,6 +15540,7 @@ export class OrcaRuntimeService { 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) diff --git a/src/relay/git-handler-worktree-ops.test.ts b/src/relay/git-handler-worktree-ops.test.ts index bd12d067bcb..d8ea6d51811 100644 --- a/src/relay/git-handler-worktree-ops.test.ts +++ b/src/relay/git-handler-worktree-ops.test.ts @@ -287,6 +287,16 @@ describe('removeWorktreeOp', () => { ) }) + 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 552eb54cf19..fcbf2608168 100644 --- a/src/relay/git-handler-worktree-remove.ts +++ b/src/relay/git-handler-worktree-remove.ts @@ -1,6 +1,9 @@ import * as path from 'node:path' import type { RemoveWorktreeResult } from '../shared/types' -import { assertWorktreeUnlockedForRemoval } from '../shared/worktree-removal' +import { + assertWorktreeRemovalForcePermissions, + 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,6 +163,7 @@ export async function removeWorktreeOp( const overrideLock = params.overrideLock === true const deleteBranch = params.deleteBranch !== false const forceBranchDelete = params.forceBranchDelete === true + assertWorktreeRemovalForcePermissions(force === true, overrideLock) let repoPath = worktreePath try { diff --git a/src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts b/src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts index 30271b337e8..3a0f709caae 100644 --- a/src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts +++ b/src/renderer/src/components/sidebar/delete-worktree-dirty-change-counts.ts @@ -23,6 +23,8 @@ export function getDeleteWorktreeDirtyChangeCounts({ if ((changeCount ?? 0) > 0) { result.set(item.id, changeCount ?? 0) } else if (forceDeleteReason === 'dirty') { + // Why: Git proved the worktree dirty even when renderer status has not + // loaded; keep the warning visible without inventing a file count. result.set(item.id, 0) } } diff --git a/src/shared/worktree-removal.ts b/src/shared/worktree-removal.ts index c3b3d597c2b..5505ff9b7b8 100644 --- a/src/shared/worktree-removal.ts +++ b/src/shared/worktree-removal.ts @@ -1,6 +1,8 @@ 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' @@ -26,6 +28,14 @@ export function assertWorktreeUnlockedForRemoval( } } +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 getLockedWorktreeRemovalReason(error: string): string | null { const prefixIndex = error.indexOf(`${LOCKED_WORKTREE_REMOVAL_PREFIX} Lock reason: `) if (prefixIndex === -1) {