Require force permission for lock overrides

This commit is contained in:
Jinwoo Hong
2026-07-10 02:12:35 -04:00
parent 6489166c2e
commit fc65a6aa07
10 changed files with 78 additions and 4 deletions
+8
View File
@@ -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': {
+5 -1
View File
@@ -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<RemoveWorktreeResult> {
assertWorktreeRemovalForcePermissions(force, options.overrideLock)
const removedWorktree =
options.knownRemovedWorktree ??
(await listWorktrees(repoPath, options)).find((worktree) =>
+13
View File
@@ -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([
{
+5 -1
View File
@@ -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) {
+15
View File
@@ -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 = {
+5 -1
View File
@@ -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)
@@ -287,6 +287,16 @@ describe('removeWorktreeOp', () => {
)
})
it('rejects SSH lock override without dirty-file force before invoking Git', async () => {
const git = vi.fn<GitExec>()
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<GitExec>(async (args, cwd) => {
+5 -1
View File
@@ -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 {
@@ -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)
}
}
+10
View File
@@ -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) {