fix(worktrees): force-remove clean worktree with initialised submodule (#9096)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: MarkXian <mark-xian@foxmail.com>
This commit is contained in:
parent
6e91ca6c0e
commit
d363c83ae3
|
|
@ -280,6 +280,121 @@ branch refs/heads/main
|
|||
expect(getGitCalls()).toContain('git worktree remove --force /repo-feature')
|
||||
})
|
||||
|
||||
it('force-retries removal when git refuses a clean worktree containing an initialised submodule', 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
|
||||
`
|
||||
},
|
||||
'git worktree list --porcelain#2': {
|
||||
stdout: `worktree /repo
|
||||
HEAD abc123
|
||||
branch refs/heads/main
|
||||
`
|
||||
},
|
||||
'git worktree remove /repo-feature': {
|
||||
error: new Error('git worktree remove failed'),
|
||||
stderr: 'fatal: working trees containing submodules cannot be moved or removed'
|
||||
},
|
||||
'git status --porcelain --untracked-files=all': { stdout: '' }
|
||||
})
|
||||
|
||||
await removeWorktree('/repo', '/repo-feature')
|
||||
|
||||
const calls = getGitCalls()
|
||||
expectGitCallOrder(
|
||||
calls,
|
||||
'git worktree remove /repo-feature',
|
||||
'git status --porcelain --untracked-files=all'
|
||||
)
|
||||
expectGitCallOrder(
|
||||
calls,
|
||||
'git status --porcelain --untracked-files=all',
|
||||
'git worktree remove --force /repo-feature'
|
||||
)
|
||||
expect(calls).toContain('git branch -d -- feature/test')
|
||||
})
|
||||
|
||||
it('surfaces uncommitted changes instead of force-removing a dirty submodule worktree', 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
|
||||
`
|
||||
},
|
||||
'git worktree remove /repo-feature': {
|
||||
error: new Error('git worktree remove failed'),
|
||||
stderr: 'fatal: working trees containing submodules cannot be moved or removed'
|
||||
},
|
||||
'git status --porcelain --untracked-files=all': { stdout: ' M sub\n' }
|
||||
})
|
||||
|
||||
await expect(removeWorktree('/repo', '/repo-feature')).rejects.toThrow(
|
||||
'Worktree has uncommitted or untracked changes.'
|
||||
)
|
||||
expect(getGitCalls()).not.toContain('git worktree remove --force /repo-feature')
|
||||
})
|
||||
|
||||
it('does not force-retry when the caller already forced removal', 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
|
||||
`
|
||||
},
|
||||
'git worktree remove --force /repo-feature': {
|
||||
error: new Error('git worktree remove failed'),
|
||||
stderr: 'fatal: working trees containing submodules cannot be moved or removed'
|
||||
}
|
||||
})
|
||||
|
||||
await expect(removeWorktree('/repo', '/repo-feature', true)).rejects.toThrow()
|
||||
expect(
|
||||
getGitCalls().filter((call) => call === 'git worktree remove --force /repo-feature')
|
||||
).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('does not force-retry unrelated non-force remove failures', 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
|
||||
`
|
||||
},
|
||||
'git worktree remove /repo-feature': {
|
||||
error: new Error('git worktree remove failed'),
|
||||
stderr: 'fatal: contains modified or untracked files, use --force to delete it'
|
||||
}
|
||||
})
|
||||
|
||||
await expect(removeWorktree('/repo', '/repo-feature')).rejects.toThrow(
|
||||
'git worktree remove failed'
|
||||
)
|
||||
expect(getGitCalls()).not.toContain('git worktree remove --force /repo-feature')
|
||||
expect(getGitCalls()).not.toContain('git status --porcelain --untracked-files=all')
|
||||
})
|
||||
|
||||
it('rejects a locked worktree with stable app-owned copy before invoking remove', async () => {
|
||||
mockGitCommands({
|
||||
'git worktree list --porcelain': {
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import type {
|
|||
RemoveWorktreeResult
|
||||
} from '../../shared/types'
|
||||
import { assertWorktreeUnlockedForRemoval } from '../../shared/worktree-removal'
|
||||
import { isSubmoduleWorktreeRemovalRefusal } from '../../shared/worktree-submodule-removal'
|
||||
import { decodeGitCQuotedPath } from '../../shared/git-cquoted-path'
|
||||
import { parseGitRevListAheadBehindCounts } from '../../shared/git-rev-list-output'
|
||||
import { parseWslUncPath } from '../../shared/wsl-paths'
|
||||
|
|
@ -1221,7 +1222,21 @@ async function performRemoveWorktree(
|
|||
args.push('--force')
|
||||
}
|
||||
args.push(worktreePath)
|
||||
await gitExecFileAsync(args, gitExecOptions(repoPath, options))
|
||||
try {
|
||||
await gitExecFileAsync(args, gitExecOptions(repoPath, options))
|
||||
} catch (error) {
|
||||
if (force || !isSubmoduleWorktreeRemovalRefusal(error)) {
|
||||
throw error
|
||||
}
|
||||
// Why: Git refuses non-force removal of any worktree with an initialised
|
||||
// submodule even when everything is clean. Re-prove cleanliness (parent
|
||||
// status reports dirty submodule content as ` M <sub>`), then --force.
|
||||
await assertWorktreeCleanForRemoval(worktreePath, false, options)
|
||||
await gitExecFileAsync(
|
||||
['worktree', 'remove', '--force', worktreePath],
|
||||
gitExecOptions(repoPath, options)
|
||||
)
|
||||
}
|
||||
|
||||
if (!branchName) {
|
||||
return {}
|
||||
|
|
|
|||
|
|
@ -196,6 +196,117 @@ describe('removeWorktreeOp', () => {
|
|||
])
|
||||
})
|
||||
|
||||
it('force-retries removal when git refuses a clean worktree containing an initialised submodule', async () => {
|
||||
const calls: string[] = []
|
||||
let listCount = 0
|
||||
const git = vi.fn<GitExec>(async (args, cwd) => {
|
||||
calls.push(`${cwd}$ ${args.join(' ')}`)
|
||||
if (args[0] === 'rev-parse') {
|
||||
return { stdout: '/repo/.git\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'worktree' && args[1] === 'list') {
|
||||
listCount += 1
|
||||
return {
|
||||
stdout:
|
||||
listCount === 1
|
||||
? worktreeList(
|
||||
{ path: '/repo', branch: 'main' },
|
||||
{ path: '/repo-feature', branch: 'feature/test' }
|
||||
)
|
||||
: worktreeList({ path: '/repo', branch: 'main' }),
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
if (args[0] === 'worktree' && args[1] === 'remove' && !args.includes('--force')) {
|
||||
throw Object.assign(new Error('git worktree remove failed'), {
|
||||
stderr: 'fatal: working trees containing submodules cannot be moved or removed'
|
||||
})
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
await removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' })
|
||||
|
||||
expect(calls).toEqual([
|
||||
'/repo-feature$ rev-parse --git-common-dir',
|
||||
`${resolvedRepoPath()}$ worktree list --porcelain -z`,
|
||||
`${resolvedRepoPath()}$ worktree remove /repo-feature`,
|
||||
'/repo-feature$ status --porcelain --untracked-files=all',
|
||||
`${resolvedRepoPath()}$ worktree remove --force /repo-feature`,
|
||||
`${resolvedRepoPath()}$ branch -d -- feature/test`
|
||||
])
|
||||
})
|
||||
|
||||
it('surfaces uncommitted changes instead of force-removing a dirty submodule worktree', async () => {
|
||||
const git = vi.fn<GitExec>(async (args) => {
|
||||
if (args[0] === 'rev-parse') {
|
||||
return { stdout: '/repo/.git\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'worktree' && args[1] === 'list') {
|
||||
return {
|
||||
stdout: worktreeList(
|
||||
{ path: '/repo', branch: 'main' },
|
||||
{ path: '/repo-feature', branch: 'feature/test' }
|
||||
),
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
if (args[0] === 'worktree' && args[1] === 'remove') {
|
||||
throw Object.assign(new Error('git worktree remove failed'), {
|
||||
stderr: 'fatal: working trees containing submodules cannot be moved or removed'
|
||||
})
|
||||
}
|
||||
if (args[0] === 'status') {
|
||||
return { stdout: ' M sub\n', stderr: '' }
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
await expect(
|
||||
removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' })
|
||||
).rejects.toThrow('Worktree has uncommitted or untracked changes.')
|
||||
expect(git).not.toHaveBeenCalledWith(
|
||||
['worktree', 'remove', '--force', '/repo-feature'],
|
||||
expect.any(String)
|
||||
)
|
||||
})
|
||||
|
||||
it('does not force-retry when the caller already forced SSH removal', async () => {
|
||||
const git = vi.fn<GitExec>(async (args) => {
|
||||
if (args[0] === 'rev-parse') {
|
||||
return { stdout: '/repo/.git\n', stderr: '' }
|
||||
}
|
||||
if (args[0] === 'worktree' && args[1] === 'list') {
|
||||
return {
|
||||
stdout: worktreeList(
|
||||
{ path: '/repo', branch: 'main' },
|
||||
{ path: '/repo-feature', branch: 'feature/test' }
|
||||
),
|
||||
stderr: ''
|
||||
}
|
||||
}
|
||||
if (args[0] === 'worktree' && args[1] === 'remove') {
|
||||
throw Object.assign(new Error('git worktree remove failed'), {
|
||||
stderr: 'fatal: working trees containing submodules cannot be moved or removed'
|
||||
})
|
||||
}
|
||||
return { stdout: '', stderr: '' }
|
||||
})
|
||||
|
||||
await expect(
|
||||
removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature', force: true })
|
||||
).rejects.toThrow('git worktree remove failed')
|
||||
expect(
|
||||
git.mock.calls.filter(
|
||||
([args]) => args[0] === 'worktree' && args[1] === 'remove' && args.includes('--force')
|
||||
)
|
||||
).toHaveLength(1)
|
||||
expect(git).not.toHaveBeenCalledWith(
|
||||
['status', '--porcelain', '--untracked-files=all'],
|
||||
expect.any(String)
|
||||
)
|
||||
})
|
||||
|
||||
it('preserves the branch (does not throw) when `branch -d` refuses an unmerged branch', async () => {
|
||||
let listCount = 0
|
||||
const git = vi.fn<GitExec>(async (args) => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import * as path from 'node:path'
|
||||
import type { RemoveWorktreeResult } from '../shared/types'
|
||||
import { assertWorktreeUnlockedForRemoval } from '../shared/worktree-removal'
|
||||
import { isSubmoduleWorktreeRemovalRefusal } from '../shared/worktree-submodule-removal'
|
||||
import { deleteAlreadyMergedRelayBranchAfterSafeDeleteFailure } from './git-handler-branch-cleanup'
|
||||
import type { GitExec } from './git-handler-ops'
|
||||
import type { GitCapabilityCache } from '../shared/git-capability-cache'
|
||||
|
|
@ -156,7 +157,23 @@ export async function removeWorktreeOp(
|
|||
args.push('--force')
|
||||
}
|
||||
args.push(worktreePath)
|
||||
await git(args, repoPath)
|
||||
try {
|
||||
await git(args, repoPath)
|
||||
} catch (error) {
|
||||
if (force || !isSubmoduleWorktreeRemovalRefusal(error)) {
|
||||
throw error
|
||||
}
|
||||
// Why: Git refuses non-force removal of any worktree with an initialised
|
||||
// submodule even when everything is clean. Re-prove cleanliness (parent
|
||||
// status reports dirty submodule content as ` M <sub>`), then --force.
|
||||
const { stdout } = await git(['status', '--porcelain', '--untracked-files=all'], worktreePath)
|
||||
if (stdout.trim()) {
|
||||
const dirtyError = new Error('Worktree has uncommitted or untracked changes.')
|
||||
;(dirtyError as Error & { stdout?: string }).stdout = stdout
|
||||
throw dirtyError
|
||||
}
|
||||
await git(['worktree', 'remove', '--force', worktreePath], repoPath)
|
||||
}
|
||||
|
||||
if (!branchName) {
|
||||
return {}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,39 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { isSubmoduleWorktreeRemovalRefusal } from './worktree-submodule-removal'
|
||||
|
||||
describe('isSubmoduleWorktreeRemovalRefusal', () => {
|
||||
it('matches the English git fatal on stderr', () => {
|
||||
expect(
|
||||
isSubmoduleWorktreeRemovalRefusal(
|
||||
Object.assign(new Error('git worktree remove failed'), {
|
||||
stderr: 'fatal: working trees containing submodules cannot be moved or removed\n'
|
||||
})
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('matches when the refusal is only in the error message', () => {
|
||||
expect(
|
||||
isSubmoduleWorktreeRemovalRefusal(
|
||||
new Error('fatal: working trees containing submodules cannot be moved or removed')
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('does not match dirty-worktree or lock refusals', () => {
|
||||
expect(
|
||||
isSubmoduleWorktreeRemovalRefusal(
|
||||
Object.assign(new Error('git worktree remove failed'), {
|
||||
stderr: 'fatal: contains modified or untracked files, use --force to delete it'
|
||||
})
|
||||
)
|
||||
).toBe(false)
|
||||
expect(
|
||||
isSubmoduleWorktreeRemovalRefusal(
|
||||
Object.assign(new Error('git worktree remove failed'), {
|
||||
stderr: 'fatal: cannot remove a locked working tree'
|
||||
})
|
||||
)
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
function getErrorText(error: unknown): string {
|
||||
if (typeof error === 'object' && error !== null) {
|
||||
const parts: string[] = []
|
||||
for (const field of ['message', 'stderr', 'stdout'] as const) {
|
||||
const value = (error as Record<string, unknown>)[field]
|
||||
if (typeof value === 'string' && value) {
|
||||
parts.push(value)
|
||||
}
|
||||
}
|
||||
return parts.join('\n')
|
||||
}
|
||||
return String(error)
|
||||
}
|
||||
|
||||
// Why: `git worktree remove` (non-force) categorically refuses any worktree
|
||||
// containing an initialised submodule, even when parent and submodule are
|
||||
// fully clean (validate_no_submodules, Git >= 2.17). Callers re-prove
|
||||
// cleanliness and retry with --force. Both the local runner and the relay pin
|
||||
// English git output (UNTRANSLATED_GIT_OUTPUT_ENV), so text matching is stable.
|
||||
export function isSubmoduleWorktreeRemovalRefusal(error: unknown): boolean {
|
||||
return /working trees containing submodules cannot be moved or removed/i.test(getErrorText(error))
|
||||
}
|
||||
Loading…
Reference in New Issue