starred/orca
1
0
Fork 0
mirror of https://github.com/stablyai/orca.git synced 2026-10-17 00:03:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

Recover Windows worktree deletion after Git deregisters

Browse Source
This commit is contained in:
Jinwoo Hong
2026-07-10 00:59:48 -04:00
parent c4c0266d0c
commit 140615a11f
5 changed files with 280 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/ipc/worktrees.ts
+4 -4
View File
@@ -132,7 +132,7 @@ import {
} from '../local-worktree-filesystem'
import {
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval,
recoverLocalWindowsLongPathWorktreeRemoval
recoverLocalWindowsWorktreeRemoval
} from '../local-worktree-removal-recovery'
const WORKTREE_ARCHIVE_HOOK_TIMEOUT_MS = 120_000
@@ -1766,9 +1766,9 @@ export function registerWorktreeHandlers(
refreshedRegisteredWorktree.head
)
} catch (error) {
// Why: Git for Windows can fail long-path directory deletion after
// Orca has already validated the target and explicit force delete.
const recoveredRemovalResult = await recoverLocalWindowsLongPathWorktreeRemoval({
// Why: Git for Windows can deregister a clean worktree before its
// recursive filesystem deletion fails transiently.
const recoveredRemovalResult = await recoverLocalWindowsWorktreeRemoval({
error,
force: args.force ?? false,
canonicalWorktreePath,
src/main/local-worktree-removal-recovery-live.test.ts
+129
View File
@@ -0,0 +1,129 @@
import { spawn, spawnSync, type ChildProcess } from 'node:child_process'
import { existsSync } from 'node:fs'
import { mkdtemp, rm, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import path from 'node:path'
import { setTimeout as delay } from 'node:timers/promises'
import { afterEach, describe, expect, it } from 'vitest'
import { recoverLocalWindowsWorktreeRemoval } from './local-worktree-removal-recovery'
const tempRoots: string[] = []
function git(cwd: string, args: string[]): string {
const result = spawnSync('git', args, { cwd, encoding: 'utf8' })
if (result.status !== 0) {
throw new Error(result.stderr || result.stdout || `git ${args.join(' ')} failed`)
}
return result.stdout.trim()
}
async function waitForPath(targetPath: string): Promise<void> {
const deadline = Date.now() + 5_000
while (!existsSync(targetPath)) {
if (Date.now() >= deadline) {
throw new Error(`Timed out waiting for ${targetPath}`)
}
await delay(20)
}
}
async function stopProcess(child: ChildProcess): Promise<void> {
if (child.exitCode !== null || child.signalCode !== null) {
return
}
const exited = new Promise<void>((resolve) => child.once('exit', () => resolve()))
child.kill()
await Promise.race([exited, delay(500)])
if (child.exitCode === null && child.pid) {
spawnSync('taskkill', ['/pid', String(child.pid), '/t', '/f'], { stdio: 'ignore' })
await Promise.race([exited, delay(2_000)])
}
if (child.exitCode === null && child.signalCode === null) {
throw new Error('Failed to stop the filesystem churn process')
}
}
describe('local Windows worktree removal recovery (live Git)', () => {
afterEach(async () => {
await Promise.all(
tempRoots
.splice(0)
.map((root) => rm(root, { recursive: true, force: true, maxRetries: 8, retryDelay: 100 }))
)
})
it.runIf(process.platform === 'win32')(
'finishes deletion when Git deregisters before a directory-not-empty failure',
async () => {
const root = await mkdtemp(path.join(tmpdir(), 'orca-worktree-enotempty-'))
tempRoots.push(root)
const repoPath = path.join(root, 'repo')
const worktreePath = path.join(root, 'worktree')
git(root, ['init', '--quiet', repoPath])
git(repoPath, ['config', 'user.email', 'test@example.com'])
git(repoPath, ['config', 'user.name', 'Test User'])
await writeFile(path.join(repoPath, '.gitignore'), 'churn/\n')
git(repoPath, ['add', '.gitignore'])
git(repoPath, ['commit', '--quiet', '-m', 'initial'])
git(repoPath, ['worktree', 'add', '--quiet', '-b', 'feature/churn', worktreePath])
const head = git(worktreePath, ['rev-parse', 'HEAD'])
// Why: a producer racing Git's recursive delete deterministically exposes
// the Windows ENOTEMPTY state reported by real workspace deletions.
const churn = spawn(
process.execPath,
[
'-e',
[
"const fs = require('node:fs')",
"const path = require('node:path')",
'const root = process.argv[1]',
'let i = 0',
'while (fs.existsSync(root)) {',
' try {',
" const dir = path.join(root, 'churn')",
' fs.mkdirSync(dir, { recursive: true })',
" fs.writeFileSync(path.join(dir, 'f' + (i++ % 64) + '.txt'), 'x')",
' } catch {}',
'}'
].join('\n'),
worktreePath
],
{ stdio: 'ignore' }
)
try {
await waitForPath(path.join(worktreePath, 'churn'))
const removal = spawnSync('git', ['worktree', 'remove', worktreePath], {
cwd: repoPath,
encoding: 'utf8'
})
expect(removal.status).not.toBe(0)
expect(git(repoPath, ['worktree', 'list', '--porcelain'])).not.toContain(worktreePath)
expect(existsSync(worktreePath)).toBe(true)
await stopProcess(churn)
await expect(
recoverLocalWindowsWorktreeRemoval({
error: Object.assign(new Error('git worktree remove failed'), {
stderr: removal.stderr
}),
force: false,
canonicalWorktreePath: worktreePath,
repoPath,
localWorktreeGitOptions: {},
registeredWorktree: { branch: 'refs/heads/feature/churn', head },
deleteBranch: false,
overrideLock: false,
closeWatcher: async () => {}
})
).resolves.toEqual({})
expect(existsSync(worktreePath)).toBe(false)
} finally {
await stopProcess(churn)
}
}
)
})
src/main/local-worktree-removal-recovery.test.ts
+92 -5
View File
@@ -26,7 +26,7 @@ vi.mock('./git/worktree', () => ({
}))
import {
recoverLocalWindowsLongPathWorktreeRemoval,
recoverLocalWindowsWorktreeRemoval,
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval
} from './local-worktree-removal-recovery'
@@ -40,7 +40,7 @@ async function withPlatform<T>(platform: NodeJS.Platform, fn: () => Promise<T>):
}
}
describe('recoverLocalWindowsLongPathWorktreeRemoval', () => {
describe('recoverLocalWindowsWorktreeRemoval', () => {
beforeEach(() => {
gitExecFileAsyncMock.mockReset()
listWorktreesStrictMock.mockReset()
@@ -62,7 +62,7 @@ describe('recoverLocalWindowsLongPathWorktreeRemoval', () => {
stderr: "error: failed to delete 'C:/repo/worktree/delete-e2e-held-cwd': Permission denied"
})
const result = await recoverLocalWindowsLongPathWorktreeRemoval({
const result = await recoverLocalWindowsWorktreeRemoval({
error,
force: true,
canonicalWorktreePath: 'C:/repo/worktree/delete-e2e-held-cwd',
@@ -90,6 +90,93 @@ describe('recoverLocalWindowsLongPathWorktreeRemoval', () => {
})
})
it('finishes a clean non-force deletion after Git leaves a Windows directory behind', async () => {
await withPlatform('win32', async () => {
const error = Object.assign(new Error('git worktree remove failed'), {
stderr:
"error: failed to delete 'C:/repo/worktree/delete-e2e-held-cwd': Directory not empty"
})
const result = await recoverLocalWindowsWorktreeRemoval({
error,
force: false,
canonicalWorktreePath: 'C:/repo/worktree/delete-e2e-held-cwd',
repoPath: 'C:/repo',
localWorktreeGitOptions: {},
registeredWorktree: { branch: 'refs/heads/delete-e2e-held-cwd', head: 'abc123' },
deleteBranch: true,
overrideLock: false,
closeWatcher: vi.fn().mockResolvedValue(undefined)
})
expect(removeLocalWorktreePathMock).toHaveBeenCalledWith(
'C:/repo/worktree/delete-e2e-held-cwd',
{}
)
expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['worktree', 'prune'], {
cwd: 'C:/repo'
})
expect(result).toEqual({
preservedBranch: {
branchName: 'delete-e2e-held-cwd',
head: 'abc123'
}
})
})
})
it('recovers localized Windows failures after Git already removed the registration', async () => {
await withPlatform('win32', async () => {
const result = await recoverLocalWindowsWorktreeRemoval({
error: Object.assign(new Error('git worktree remove failed'), {
stderr: "Fehler: 'C:/repo/worktree/feature' konnte nicht gelöscht werden"
}),
force: false,
canonicalWorktreePath: 'C:/repo/worktree/feature',
repoPath: 'C:/repo',
localWorktreeGitOptions: {},
registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' },
deleteBranch: false,
overrideLock: false,
closeWatcher: vi.fn().mockResolvedValue(undefined)
})
expect(removeLocalWorktreePathMock).toHaveBeenCalledWith('C:/repo/worktree/feature', {})
expect(result).toEqual({})
})
})
it('does not recover unrelated localized failures while Git still owns the row', async () => {
await withPlatform('win32', async () => {
listWorktreesStrictMock.mockResolvedValue([
{
path: 'C:/repo/worktree/feature',
head: 'abc123',
branch: 'refs/heads/feature',
isBare: false,
isMainWorktree: false
}
])
await expect(
recoverLocalWindowsWorktreeRemoval({
error: Object.assign(new Error('git worktree remove failed'), {
stderr: 'Fehler: unerwarteter Git-Zustand'
}),
force: false,
canonicalWorktreePath: 'C:/repo/worktree/feature',
repoPath: 'C:/repo',
localWorktreeGitOptions: {},
registeredWorktree: { branch: 'refs/heads/feature', head: 'abc123' },
deleteBranch: false,
overrideLock: false,
closeWatcher: vi.fn().mockResolvedValue(undefined)
})
).resolves.toBeUndefined()
expect(removeLocalWorktreePathMock).not.toHaveBeenCalled()
})
})
it('does not recover partial filesystem deletion wording off Windows', async () => {
await withPlatform('linux', async () => {
const error = Object.assign(new Error('git worktree remove failed'), {
@@ -97,7 +184,7 @@ describe('recoverLocalWindowsLongPathWorktreeRemoval', () => {
})
await expect(
recoverLocalWindowsLongPathWorktreeRemoval({
recoverLocalWindowsWorktreeRemoval({
error,
force: true,
canonicalWorktreePath: 'C:/repo/worktree/delete-e2e-held-cwd',
@@ -122,7 +209,7 @@ describe('recoverLocalWindowsLongPathWorktreeRemoval', () => {
lockReason: 'active agent'
}
await recoverLocalWindowsLongPathWorktreeRemoval({
await recoverLocalWindowsWorktreeRemoval({
error: Object.assign(new Error('git worktree remove failed'), {
stderr: 'error: failed to delete deep/file.txt: Filename too long'
}),
src/main/local-worktree-removal-recovery.ts
+51 -23
View File
@@ -9,7 +9,7 @@ import { gitExecFileAsync } from './git/runner'
import { listWorktreesStrict, removeWorktree, type GitWorktreeExecOptions } from './git/worktree'
import { removeLocalWorktreePath } from './local-worktree-filesystem'
type LocalWindowsLongPathRecoveryArgs = {
type LocalWindowsRemovalRecoveryArgs = {
error: unknown
force: boolean
canonicalWorktreePath: string
@@ -22,7 +22,7 @@ type LocalWindowsLongPathRecoveryArgs = {
}
type StaleLocalWorktreeRegistrationArgs = Omit<
LocalWindowsLongPathRecoveryArgs,
LocalWindowsRemovalRecoveryArgs,
'error' | 'force' | 'closeWatcher'
>
@@ -41,12 +41,16 @@ function preservedBranchResult(
}
}
function staleRegistrationRecoveryError(error: unknown, canonicalWorktreePath: string): Error {
function staleRegistrationRecoveryError(
error: unknown,
canonicalWorktreePath: string,
force: boolean
): Error {
return new Error(
`${formatWorktreeRemovalError(
error,
canonicalWorktreePath,
true
force
)} The worktree directory was removed, but Git still has stale worktree registration. Retry deletion after resolving the Git registration error.`
)
}
@@ -54,7 +58,8 @@ function staleRegistrationRecoveryError(error: unknown, canonicalWorktreePath: s
async function verifyGitWorktreeRegistrationRemoved(
repoPath: string,
localWorktreeGitOptions: GitWorktreeExecOptions,
canonicalWorktreePath: string
canonicalWorktreePath: string,
force: boolean
): Promise<void> {
try {
const remainingWorktrees = await listWorktreesStrict(repoPath, localWorktreeGitOptions)
@@ -66,12 +71,13 @@ async function verifyGitWorktreeRegistrationRemoved(
throw new Error('Git still reports the worktree registration after cleanup.')
}
} catch (error) {
throw staleRegistrationRecoveryError(error, canonicalWorktreePath)
throw staleRegistrationRecoveryError(error, canonicalWorktreePath, force)
}
}
async function removeRequiredGitWorktreeRegistration(
args: StaleLocalWorktreeRegistrationArgs
args: StaleLocalWorktreeRegistrationArgs,
forceForError = true
): Promise<RemoveWorktreeResult> {
assertWorktreeUnlockedForRemoval(args.registeredWorktree, args.overrideLock)
@@ -104,11 +110,12 @@ async function removeRequiredGitWorktreeRegistration(
await verifyGitWorktreeRegistrationRemoved(
args.repoPath,
args.localWorktreeGitOptions,
args.canonicalWorktreePath
args.canonicalWorktreePath,
forceForError
)
} catch (verificationError) {
throw removalError
? staleRegistrationRecoveryError(removalError, args.canonicalWorktreePath)
? staleRegistrationRecoveryError(removalError, args.canonicalWorktreePath, forceForError)
: verificationError
}
// Why: if Git detached the row before reporting its filesystem error, keep
@@ -116,38 +123,59 @@ async function removeRequiredGitWorktreeRegistration(
return result ?? preservedBranchResult(args.registeredWorktree, args.deleteBranch)
}
export async function recoverLocalWindowsLongPathWorktreeRemoval(
args: LocalWindowsLongPathRecoveryArgs
export async function recoverLocalWindowsWorktreeRemoval(
args: LocalWindowsRemovalRecoveryArgs
): Promise<RemoveWorktreeResult | undefined> {
if (!args.force || !isRecoverableWindowsFilesystemRemovalError(args.error)) {
if (!(await isRecoverableWindowsFilesystemRemovalFailure(args))) {
return undefined
}
// Why: watcher shutdown is best-effort, but Git registration must be removed
// before callers clear Orca metadata or the branch remains locked.
// Why: this error means Git accepted removal and started deleting the path;
// finish that partial success with Windows retries, then verify Git metadata.
await args.closeWatcher(args.canonicalWorktreePath).catch(() => {})
try {
await removeLocalWorktreePath(args.canonicalWorktreePath, args.localWorktreeGitOptions)
} catch (error) {
throw new Error(formatWorktreeRemovalError(error, args.canonicalWorktreePath, true))
throw new Error(formatWorktreeRemovalError(error, args.canonicalWorktreePath, args.force))
}
return removeRequiredGitWorktreeRegistration(args)
return removeRequiredGitWorktreeRegistration(args, args.force)
}
function isRecoverableWindowsFilesystemRemovalError(error: unknown): boolean {
if (isWindowsLongPathWorktreeRemovalError(error)) {
async function isRecoverableWindowsFilesystemRemovalFailure(
args: LocalWindowsRemovalRecoveryArgs
): Promise<boolean> {
if (isWindowsLongPathWorktreeRemovalError(args.error)) {
return true
}
if (process.platform !== 'win32' || typeof error !== 'object' || error === null) {
if (process.platform !== 'win32' || typeof args.error !== 'object' || args.error === null) {
return false
}
const errorWithDetails = error as { message?: unknown; stderr?: unknown; stdout?: unknown }
const errorWithDetails = args.error as {
message?: unknown
stderr?: unknown
stdout?: unknown
}
const details = [errorWithDetails.stderr, errorWithDetails.stdout, errorWithDetails.message]
.filter((value): value is string => typeof value === 'string' && value.trim().length > 0)
.join('\n')
return /failed to delete .*(?:directory not empty|permission denied|access is denied|being used by another process)|(?:directory not empty|permission denied|access is denied|being used by another process).*failed to delete/i.test(
details
)
if (
/failed to delete .*(?:directory not empty|permission denied|access is denied|being used by another process)|(?:directory not empty|permission denied|access is denied|being used by another process).*failed to delete/i.test(
details
)
) {
return true
}
try {
const worktrees = await listWorktreesStrict(args.repoPath, args.localWorktreeGitOptions)
// Why: Git can localize ENOTEMPTY prose, but a missing registration proves
// it already accepted removal and only Windows filesystem cleanup remains.
return !worktrees.some((worktree) =>
areWorktreePathsEqual(worktree.path, args.canonicalWorktreePath)
)
} catch {
return false
}
}
export async function removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval(
src/main/runtime/orca-runtime.ts
+4 -4
View File
@@ -450,7 +450,7 @@ import {
} from '../local-worktree-filesystem'
import {
removeStaleLocalWorktreeRegistrationAfterFilesystemRemoval,
recoverLocalWindowsLongPathWorktreeRemoval
recoverLocalWindowsWorktreeRemoval
} from '../local-worktree-removal-recovery'
import {
connect as connectLinear,
@@ -15893,9 +15893,9 @@ export class OrcaRuntimeService {
refreshedRegisteredWorktree.head
)
} catch (error) {
// Why: Git for Windows can fail long-path directory deletion after
// Orca has already validated the target and explicit force delete.
const recoveredRemovalResult = await recoverLocalWindowsLongPathWorktreeRemoval({
// Why: Git for Windows can deregister a clean worktree before its
// recursive filesystem deletion fails transiently.
const recoveredRemovalResult = await recoverLocalWindowsWorktreeRemoval({
error,
force,
canonicalWorktreePath,