Allow OS metadata during worktree delete preflight (#2760)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson 2026-05-24 23:00:40 -07:00 committed by GitHub
parent 4bacdc534d
commit fdb63366d3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 272 additions and 1 deletions

View File

@ -341,6 +341,34 @@ describe('assertWorktreeCleanForRemoval', () => {
)
})
it('removes disposable OS metadata before deciding a worktree is dirty', async () => {
gitExecFileAsyncMock
.mockResolvedValueOnce({ stdout: '?? .DS_Store\n?? nested/.DS_Store\n', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
.mockResolvedValueOnce({ stdout: '', stderr: '' })
await expect(assertWorktreeCleanForRemoval('/repo-feature')).resolves.toBeUndefined()
expect(getGitCalls()).toEqual([
'git status --porcelain --untracked-files=all',
'git clean -f -q -- .DS_Store :(glob)**/.DS_Store Thumbs.db :(glob)**/Thumbs.db Desktop.ini :(glob)**/Desktop.ini',
'git status --porcelain --untracked-files=all'
])
})
it('does not remove disposable metadata when real untracked files also block deletion', async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({
stdout: '?? .DS_Store\n?? scratch.txt\n',
stderr: ''
})
await expect(assertWorktreeCleanForRemoval('/repo-feature')).rejects.toMatchObject({
message: 'Worktree has uncommitted or untracked changes.',
stdout: '?? .DS_Store\n?? scratch.txt\n'
})
expect(getGitCalls()).toEqual(['git status --porcelain --untracked-files=all'])
})
it('throws a dedicated dirty/untracked error when status output is non-empty', async () => {
gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '?? scratch.txt\n', stderr: '' })

View File

@ -3,6 +3,10 @@ import { stat } from 'fs/promises'
import { join, posix, win32 } from 'path'
import { resolveWorktreeAddBaseRef } from '../../shared/worktree-base-ref'
import type { GitWorktreeInfo } from '../../shared/types'
import {
disposableWorktreeMetadataPathspecs,
hasOnlyDisposableWorktreeMetadata
} from '../../shared/disposable-worktree-metadata'
import { gitExecFileAsync, translateWslOutputPaths } from './runner'
import { resolveGitDir } from './status'
import { hasWorktreeBaseCommitRef } from './worktree-base-ref-probe'
@ -420,13 +424,31 @@ export async function assertWorktreeCleanForRemoval(
return
}
const { stdout } = await gitExecFileAsync(['status', '--porcelain', '--untracked-files=all'], {
let { stdout } = await gitExecFileAsync(['status', '--porcelain', '--untracked-files=all'], {
cwd: worktreePath
})
if (!stdout.trim()) {
return
}
if (hasOnlyDisposableWorktreeMetadata(stdout)) {
// Why: Finder/Explorer metadata can make a user-clean worktree require
// force-delete. Remove only untracked disposable files, then re-check.
await gitExecFileAsync(['clean', '-f', '-q', '--', ...disposableWorktreeMetadataPathspecs], {
cwd: worktreePath
})
const statusAfterCleanup = await gitExecFileAsync(
['status', '--porcelain', '--untracked-files=all'],
{
cwd: worktreePath
}
)
stdout = statusAfterCleanup.stdout
if (!stdout.trim()) {
return
}
}
const error = new Error('Worktree has uncommitted or untracked changes.')
;(error as Error & { stdout?: string }).stdout = stdout
throw error

View File

@ -44,6 +44,7 @@ describe('removeWorktreeOp', () => {
expect(calls).toEqual([
'/repo-feature$ rev-parse --git-common-dir',
'/repo$ worktree list --porcelain',
'/repo-feature$ status --porcelain --untracked-files=all',
'/repo$ worktree remove /repo-feature',
'/repo$ worktree prune',
'/repo$ worktree list --porcelain',
@ -75,11 +76,128 @@ describe('removeWorktreeOp', () => {
expect(calls).toEqual([
'/repo-feature$ rev-parse --git-common-dir',
'/repo$ worktree list --porcelain',
'/repo-feature$ status --porcelain --untracked-files=all',
'/repo$ worktree remove /repo-feature',
'/repo$ worktree prune'
])
})
it('removes disposable metadata before removing an SSH worktree', async () => {
const calls: string[] = []
let listCount = 0
let statusCount = 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] === 'status') {
statusCount += 1
return {
stdout: statusCount === 1 ? '?? .DS_Store\n?? nested/.DS_Store\n' : '',
stderr: ''
}
}
return { stdout: '', stderr: '' }
})
await removeWorktreeOp(git, { worktreePath: '/repo-feature' })
expect(calls).toEqual([
'/repo-feature$ rev-parse --git-common-dir',
'/repo$ worktree list --porcelain',
'/repo-feature$ status --porcelain --untracked-files=all',
'/repo-feature$ clean -f -q -- .DS_Store :(glob)**/.DS_Store Thumbs.db :(glob)**/Thumbs.db Desktop.ini :(glob)**/Desktop.ini',
'/repo-feature$ status --porcelain --untracked-files=all',
'/repo$ worktree remove /repo-feature',
'/repo$ worktree prune',
'/repo$ worktree list --porcelain',
'/repo$ branch -D feature/test'
])
})
it('does not remove disposable metadata when real untracked files block SSH deletion', async () => {
const calls: string[] = []
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') {
return {
stdout: worktreeList(
{ path: '/repo', branch: 'main' },
{ path: '/repo-feature', branch: 'feature/test' }
),
stderr: ''
}
}
if (args[0] === 'status') {
return { stdout: '?? .DS_Store\n?? scratch.txt\n', stderr: '' }
}
return { stdout: '', stderr: '' }
})
await expect(removeWorktreeOp(git, { worktreePath: '/repo-feature' })).rejects.toMatchObject({
message: 'Worktree has uncommitted or untracked changes.',
stdout: '?? .DS_Store\n?? scratch.txt\n'
})
expect(calls).toEqual([
'/repo-feature$ rev-parse --git-common-dir',
'/repo$ worktree list --porcelain',
'/repo-feature$ status --porcelain --untracked-files=all'
])
})
it('skips disposable metadata cleanup for forced SSH worktree removal', 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: ''
}
}
return { stdout: '', stderr: '' }
})
await removeWorktreeOp(git, { worktreePath: '/repo-feature', force: true })
expect(calls).toEqual([
'/repo-feature$ rev-parse --git-common-dir',
'/repo$ worktree list --porcelain',
'/repo$ worktree remove --force /repo-feature',
'/repo$ worktree prune',
'/repo$ worktree list --porcelain',
'/repo$ branch -D feature/test'
])
})
it('keeps the branch when another SSH worktree still uses it', async () => {
let listCount = 0
const git = vi.fn<GitExec>(async (args, _cwd) => {

View File

@ -6,6 +6,10 @@
*/
import * as path from 'path'
import { resolveWorktreeAddBaseRef } from '../shared/worktree-base-ref'
import {
disposableWorktreeMetadataPathspecs,
hasOnlyDisposableWorktreeMetadata
} from '../shared/disposable-worktree-metadata'
import type { GitExec } from './git-handler-ops'
import { parseWorktreeList } from './git-handler-utils'
@ -115,6 +119,10 @@ export async function removeWorktreeOp(
)
const branchName = normalizeLocalBranchRef(removedWorktree?.branch ?? '')
if (!force) {
await assertRelayWorktreeCleanForRemoval(git, worktreePath)
}
const args = ['worktree', 'remove']
if (force) {
args.push('--force')
@ -156,6 +164,34 @@ type RelayWorktreeInfo = {
branch?: string
}
async function assertRelayWorktreeCleanForRemoval(
git: GitExec,
worktreePath: string
): Promise<void> {
let { stdout } = await git(['status', '--porcelain', '--untracked-files=all'], worktreePath)
if (!stdout.trim()) {
return
}
if (hasOnlyDisposableWorktreeMetadata(stdout)) {
// Why: SSH worktree deletion bypasses the local preflight; clean remote
// Finder/Explorer metadata here so SSH and local delete behavior match.
await git(['clean', '-f', '-q', '--', ...disposableWorktreeMetadataPathspecs], worktreePath)
const statusAfterCleanup = await git(
['status', '--porcelain', '--untracked-files=all'],
worktreePath
)
stdout = statusAfterCleanup.stdout
if (!stdout.trim()) {
return
}
}
const error = new Error('Worktree has uncommitted or untracked changes.')
;(error as Error & { stdout?: string }).stdout = stdout
throw error
}
async function listRelayWorktrees(git: GitExec, repoPath: string): Promise<RelayWorktreeInfo[]> {
try {
const { stdout } = await git(['worktree', 'list', '--porcelain'], repoPath)

View File

@ -0,0 +1,33 @@
import { describe, expect, it } from 'vitest'
import { hasOnlyDisposableWorktreeMetadata } from './disposable-worktree-metadata'
describe('hasOnlyDisposableWorktreeMetadata', () => {
it('matches root and nested disposable metadata status lines', () => {
expect(
hasOnlyDisposableWorktreeMetadata(
'?? .DS_Store\n?? nested/.DS_Store\n?? Thumbs.db\n?? nested/Desktop.ini\n'
)
).toBe(true)
})
it('matches git-quoted disposable metadata paths', () => {
expect(hasOnlyDisposableWorktreeMetadata('?? ".DS_Store"\n?? "nested/Thumbs.db"\n')).toBe(true)
expect(hasOnlyDisposableWorktreeMetadata('?? "nested dir/.DS_Store"\n')).toBe(true)
})
it('does not treat filenames ending in disposable metadata names as disposable', () => {
expect(hasOnlyDisposableWorktreeMetadata('?? "old .DS_Store"\n')).toBe(false)
expect(hasOnlyDisposableWorktreeMetadata('?? backup Thumbs.db\n')).toBe(false)
})
it('does not match paths the cleanup pathspecs do not target', () => {
expect(hasOnlyDisposableWorktreeMetadata('?? "foo\\".DS_Store"\n')).toBe(false)
expect(hasOnlyDisposableWorktreeMetadata('?? "foo\\\\.DS_Store"\n')).toBe(false)
expect(hasOnlyDisposableWorktreeMetadata('?? .ds_store\n')).toBe(false)
expect(hasOnlyDisposableWorktreeMetadata('?? thumbs.db\n')).toBe(false)
})
it('rejects mixed disposable metadata and real untracked files', () => {
expect(hasOnlyDisposableWorktreeMetadata('?? .DS_Store\n?? scratch.txt\n')).toBe(false)
})
})

View File

@ -0,0 +1,34 @@
const disposableWorktreeMetadataFilenames = ['.DS_Store', 'Thumbs.db', 'Desktop.ini']
export const disposableWorktreeMetadataPathspecs = disposableWorktreeMetadataFilenames.flatMap(
(filename) => [filename, `:(glob)**/${filename}`]
)
export function hasOnlyDisposableWorktreeMetadata(statusOutput: string): boolean {
const statusLines = statusOutput.split(/\r?\n/).filter((line) => line.trim())
return (
statusLines.length > 0 &&
statusLines.every((line) => {
if (!line.startsWith('?? ')) {
return false
}
return isDisposableWorktreeMetadataPath(line.slice(3).trim())
})
)
}
function isDisposableWorktreeMetadataPath(statusPath: string): boolean {
const path = stripSurroundingGitStatusQuotes(statusPath)
const slashIndex = path.lastIndexOf('/')
const basename = slashIndex === -1 ? path : path.slice(slashIndex + 1)
return disposableWorktreeMetadataFilenames.includes(basename)
}
function stripSurroundingGitStatusQuotes(statusPath: string): string {
if (statusPath.startsWith('"') && statusPath.endsWith('"')) {
// Why: Git quotes the full path; escaped quotes inside are filename
// content, not basename boundaries for the cleanup pathspecs.
return statusPath.slice(1, -1)
}
return statusPath
}