diff --git a/src/main/git/remove-worktree.test.ts b/src/main/git/remove-worktree.test.ts index 32b552b3f..d90da8559 100644 --- a/src/main/git/remove-worktree.test.ts +++ b/src/main/git/remove-worktree.test.ts @@ -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: '' }) diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index ed8a9e963..8b042d24b 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -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 diff --git a/src/relay/git-handler-worktree-ops.test.ts b/src/relay/git-handler-worktree-ops.test.ts index d23eee18a..ed907867a 100644 --- a/src/relay/git-handler-worktree-ops.test.ts +++ b/src/relay/git-handler-worktree-ops.test.ts @@ -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(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(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(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(async (args, _cwd) => { diff --git a/src/relay/git-handler-worktree-ops.ts b/src/relay/git-handler-worktree-ops.ts index 7fcdc8a85..2e35f6524 100644 --- a/src/relay/git-handler-worktree-ops.ts +++ b/src/relay/git-handler-worktree-ops.ts @@ -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 { + 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 { try { const { stdout } = await git(['worktree', 'list', '--porcelain'], repoPath) diff --git a/src/shared/disposable-worktree-metadata.test.ts b/src/shared/disposable-worktree-metadata.test.ts new file mode 100644 index 000000000..679904d51 --- /dev/null +++ b/src/shared/disposable-worktree-metadata.test.ts @@ -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) + }) +}) diff --git a/src/shared/disposable-worktree-metadata.ts b/src/shared/disposable-worktree-metadata.ts new file mode 100644 index 000000000..d623b21ea --- /dev/null +++ b/src/shared/disposable-worktree-metadata.ts @@ -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 +}