diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 060beadad39f7..cbe20530e0366 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -6272,6 +6272,127 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect.each([false, true])( + "retries a Windows delete (already removed: %s)", + (alreadyRemoved) => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const pathService = yield* Path.Path; + const fileSystem = yield* FileSystem.FileSystem; + const worktreePath = pathService.join(yield* makeTmpDir("git-worktrees-"), "held"); + // Windows can report Busy after partially removing a held worktree. + let heldAttempts = 2; + const firstAttempt = yield* Deferred.make(); + const secondAttempt = yield* Deferred.make(); + const driver = yield* makeGitVcsDriverCore().pipe( + Effect.provideService(FileSystem.FileSystem, { + ...fileSystem, + remove: (target, options) => + Effect.suspend(() => { + if (target !== worktreePath || heldAttempts === 0) { + return fileSystem.remove(target, options); + } + heldAttempts--; + const held = Effect.fail( + PlatformError.systemError({ + _tag: "Busy", + module: "FileSystem", + method: "remove", + pathOrDescriptor: target, + }), + ); + return ( + alreadyRemoved && heldAttempts === 0 + ? fileSystem.remove(target, options) + : Effect.void + ).pipe( + Effect.andThen( + Deferred.succeed( + heldAttempts === 0 ? secondAttempt : firstAttempt, + undefined, + ), + ), + Effect.andThen(held), + ); + }), + }), + Effect.provide(layerServerConfig), + ); + yield* driver.createWorktree({ + cwd, + path: worktreePath, + refName: initialBranch, + newRefName: "feature/held", + }); + + const removal = yield* driver + .removeWorktree({ cwd, path: worktreePath, force: true }) + .pipe(Effect.provideService(HostProcess.Platform, "win32"), Effect.forkChild); + yield* Deferred.await(firstAttempt); + yield* TestClock.adjust("250 millis"); + yield* Deferred.await(secondAttempt); + yield* TestClock.adjust("250 millis"); + yield* Fiber.join(removal); + + assert.equal(yield* fileSystem.exists(worktreePath), false); + assert.notInclude(yield* git(cwd, ["worktree", "list", "--porcelain"]), "held"); + }), + ); + + it.effect("restores Git metadata after an interrupted Windows deletion settles", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const { initialBranch } = yield* initRepoWithCommit(cwd); + const path = yield* Path.Path; + const fileSystem = yield* FileSystem.FileSystem; + const worktreePath = path.join(yield* makeTmpDir("git-worktrees-"), "interrupted"); + const gitFile = path.join(worktreePath, ".git"); + const started = yield* Deferred.make(); + const finished = yield* Deferred.make(); + const driver = yield* makeGitVcsDriverCore().pipe( + Effect.provideService(FileSystem.FileSystem, { + ...fileSystem, + remove: (target, options) => + target !== worktreePath + ? fileSystem.remove(target, options) + : Effect.gen(function* () { + yield* Deferred.succeed(started, undefined); + yield* Deferred.await(finished); + return yield* PlatformError.systemError({ + _tag: "Busy", + module: "FileSystem", + method: "remove", + pathOrDescriptor: target, + }); + }), + }), + Effect.provide(layerServerConfig), + ); + yield* driver.createWorktree({ + cwd, + path: worktreePath, + refName: initialBranch, + newRefName: "feature/interrupted-delete", + }); + const gitContents = yield* fileSystem.readFileString(gitFile); + const removal = yield* driver + .removeWorktree({ cwd, path: worktreePath, force: true }) + .pipe(Effect.provideService(HostProcess.Platform, "win32"), Effect.forkChild); + yield* Deferred.await(started); + const cancellation = yield* Fiber.interrupt(removal).pipe( + Effect.forkChild({ startImmediately: true }), + ); + yield* TestClock.adjust("0 millis"); + // The native deletion can finish after its effect has been interrupted. + yield* fileSystem.remove(gitFile); + yield* Deferred.succeed(finished, undefined); + yield* Fiber.join(cancellation); + assert.equal(yield* fileSystem.readFileString(gitFile), gitContents); + assert.include(yield* git(cwd, ["worktree", "list", "--porcelain"]), worktreePath); + }), + ); + it.effect("keeps the junctions of a worktree that git refuses to remove", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); diff --git a/apps/server/src/vcs/worktreeRemoval.ts b/apps/server/src/vcs/worktreeRemoval.ts index f1140cdcc3a48..b6573c40b2fa2 100644 --- a/apps/server/src/vcs/worktreeRemoval.ts +++ b/apps/server/src/vcs/worktreeRemoval.ts @@ -4,10 +4,14 @@ import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; +import * as Schedule from "effect/Schedule"; import type * as GitVcsDriver from "./GitVcsDriver.ts"; const INDEX_LIST_MAX_BYTES = 64 * 1024 * 1024; +// About 10 seconds of retries while a stopped provider's processes exit. +const OPEN_FILE_RETRIES = 40; +const OPEN_FILE_RETRY_DELAY = "250 millis"; /** * Git for Windows before 2.54 follows NTFS junctions; newer Git leaves them behind. @@ -142,15 +146,29 @@ export const removeLinkedWorktree = Effect.fn("WorktreeRemoval.removeLinkedWorkt ); } - // A file held open can stop the delete partway. Putting `.git` back, when - // that works, leaves a worktree that a retry can remove. + // Only forced deletes retry: a later attempt must not discard newly written edits. + // Restore `.git` on failure or interruption so Git can retry a partial deletion. const gitFileContents = yield* fileSystem .readFileString(gitFile) .pipe(Effect.mapError(() => notAWorktree)); - yield* fileSystem.remove(worktreePath, { recursive: true }).pipe( - Effect.tapError(() => fileSystem.writeFileString(gitFile, gitFileContents).pipe(Effect.ignore)), - Effect.mapError((cause) => refuse(`failed to delete '${worktreePath}'`, cause)), + // Once the folder is gone, Git cleanup must run: nothing else can drop the metadata. + return yield* Effect.uninterruptibleMask((restore) => + restore( + fileSystem.remove(worktreePath, { recursive: true, force: true }).pipe( + // Node cannot cancel fs.rm; wait for its callback before restoring `.git`. + Effect.uninterruptible, + Effect.retry({ + times: input.force ? OPEN_FILE_RETRIES : 0, + schedule: Schedule.spaced(OPEN_FILE_RETRY_DELAY), + }), + Effect.onError(() => + fileSystem.writeFileString(gitFile, gitFileContents).pipe(Effect.ignore), + ), + Effect.mapError((cause) => refuse(`failed to delete '${worktreePath}'`, cause)), + ), + ).pipe( + // The folder is gone, so Git only drops the worktree's metadata. + Effect.andThen(input.run(["worktree", "remove", "--force", "--force", "--", worktreePath])), + ), ); - // The folder is gone, so Git only drops the worktree's metadata. - return yield* input.run(["worktree", "remove", "--force", "--force", "--", worktreePath]); });