diff --git a/.changeset/windows-worktree-cleanup.md b/.changeset/windows-worktree-cleanup.md new file mode 100644 index 0000000000..7655de250f --- /dev/null +++ b/.changeset/windows-worktree-cleanup.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Improve Windows worktree cleanup reliability when file handles are released slowly. diff --git a/packages/opencode/src/worktree/index.ts b/packages/opencode/src/worktree/index.ts index 4fe5e47527..47b5dbe124 100644 --- a/packages/opencode/src/worktree/index.ts +++ b/packages/opencode/src/worktree/index.ts @@ -353,19 +353,72 @@ export const layer: Layer.Layer< ) } + // kilocode_change start - retry transient Windows worktree lock failures + function locked(error: unknown) { + return ( + typeof error === "object" && + error !== null && + "code" in error && + ["EBUSY", "EACCES", "EPERM"].includes(String(error.code)) + ) + } + function cleanDirectory(target: string) { - const retries = process.platform === "win32" ? 30 : 5 // kilocode_change - Windows may release git worktree handles slowly - const delay = process.platform === "win32" ? 250 : 100 // kilocode_change - return Effect.promise(() => - import("fs/promises") - .then((fsp) => fsp.rm(target, { recursive: true, force: true, maxRetries: retries, retryDelay: delay })) // kilocode_change - .catch((error) => { + const retries = process.platform === "win32" ? 60 : 5 + const delay = process.platform === "win32" ? 500 : 100 + return Effect.promise(async () => { + const fsp = await import("fs/promises") + const rm = async (left: number): Promise => + fsp + .rm(target, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }) + .catch(async (error) => { + if (!locked(error)) throw error + if (left <= 1) throw error + if (process.platform === "win32") Bun.gc(true) + await Bun.sleep(delay) + return rm(left - 1) + }) + return rm(retries) + }).pipe( + Effect.catch((error) => + Effect.sync(() => { const message = errorMessage(error) throw new RemoveFailedError({ message: message || "Failed to remove git worktree directory" }) }), + ), ) } + function transient(result: GitResult) { + const text = `${result.stderr}\n${result.text}`.toLowerCase() + return [ + "ebusy", + "eacces", + "eperm", + "directory not empty", + "resource busy", + "permission denied", + "access is denied", + "process cannot access", + ].some((item) => text.includes(item)) + } + + const removeWorktree = Effect.fnUntraced(function* (root: string, target: string) { + const retries = process.platform === "win32" ? 60 : 5 + const delay = process.platform === "win32" ? 500 : 100 + for (const attempt of Array.from({ length: retries }, (_, i) => i)) { + yield* stopFsmonitor(target) + const result = yield* git(["worktree", "remove", "--force", target], { cwd: root }) + if (result.code === 0) return result + if (!transient(result)) return result + if (attempt === retries - 1) return result + if (process.platform === "win32") yield* Effect.sync(() => Bun.gc(true)) + yield* Effect.sleep(`${delay} millis`) + } + return { code: 1, text: "", stderr: "Failed to remove git worktree" } satisfies GitResult + }) + // kilocode_change end + const remove = Effect.fn("Worktree.remove")(function* (input: RemoveInput) { const ctx = yield* InstanceState.context if (ctx.project.vcs !== "git") { @@ -391,8 +444,7 @@ export const layer: Layer.Layer< return true } - yield* stopFsmonitor(entry.path) - const removed = yield* git(["worktree", "remove", "--force", entry.path], { cwd: ctx.worktree }) + const removed = yield* removeWorktree(ctx.worktree, entry.path) // kilocode_change if (removed.code !== 0) { const next = yield* git(["worktree", "list", "--porcelain"], { cwd: ctx.worktree }) if (next.code !== 0) { diff --git a/packages/opencode/test/fixture/fixture.ts b/packages/opencode/test/fixture/fixture.ts index 68520b2c45..cbfceaa535 100644 --- a/packages/opencode/test/fixture/fixture.ts +++ b/packages/opencode/test/fixture/fixture.ts @@ -23,15 +23,39 @@ function exists(dir: string) { .catch(() => false) } -function clean(dir: string) { - return fs.rm(dir, { - recursive: true, - force: true, - maxRetries: 5, - retryDelay: 100, - }) +// kilocode_change start - retry Windows lock-style cleanup failures +function locked(error: unknown) { + return ( + typeof error === "object" && + error !== null && + "code" in error && + ["EBUSY", "EACCES", "EPERM"].includes(String(error.code)) + ) } +function clean(dir: string) { + const retries = process.platform === "win32" ? 60 : 5 + const delay = process.platform === "win32" ? 500 : 100 + const rm = async (left: number): Promise => { + if (process.platform === "win32") Bun.gc(true) + return fs + .rm(dir, { + recursive: true, + force: true, + maxRetries: 5, + retryDelay: 100, + }) + .catch(async (error) => { + if (!locked(error)) throw error + if (left <= 1) throw error + await Bun.sleep(delay) + return rm(left - 1) + }) + } + return rm(retries) +} +// kilocode_change end + async function stop(dir: string) { if (!(await exists(dir))) return await $`git fsmonitor--daemon stop`.cwd(dir).quiet().nothrow() diff --git a/packages/opencode/test/preload.ts b/packages/opencode/test/preload.ts index f9a61caa9a..7ecfd6d2d8 100644 --- a/packages/opencode/test/preload.ts +++ b/packages/opencode/test/preload.ts @@ -12,21 +12,26 @@ await fs.mkdir(dir, { recursive: true }) afterAll(async () => { const { Database } = await import("../src/storage") Database.close() - const busy = (error: unknown) => - typeof error === "object" && error !== null && "code" in error && error.code === "EBUSY" + // kilocode_change start - retry Windows lock-style cleanup failures + const locked = (error: unknown) => + typeof error === "object" && + error !== null && + "code" in error && + ["EBUSY", "EACCES", "EPERM"].includes(String(error.code)) const rm = async (left: number): Promise => { Bun.gc(true) - await sleep(100) - return fs.rm(dir, { recursive: true, force: true }).catch((error) => { - if (!busy(error)) throw error + await sleep(250) + return fs.rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }).catch((error) => { + if (!locked(error)) throw error if (left <= 1) throw error return rm(left - 1) }) } // Windows can keep SQLite WAL handles alive until GC finalizers run, so we - // force GC and retry teardown to avoid flaky EBUSY in test cleanup. - await rm(30) + // force GC and retry teardown to avoid flaky lock errors in test cleanup. + await rm(60) + // kilocode_change end }) process.env["XDG_DATA_HOME"] = path.join(dir, "share") diff --git a/packages/opencode/test/project/worktree-remove.test.ts b/packages/opencode/test/project/worktree-remove.test.ts index 5fb2beb286..703ec3a497 100644 --- a/packages/opencode/test/project/worktree-remove.test.ts +++ b/packages/opencode/test/project/worktree-remove.test.ts @@ -84,6 +84,77 @@ describe("Worktree.remove", () => { ), ) + // kilocode_change start - cover transient Windows worktree lock failures + it.live("retries transient git remove lock failures", () => + provideTmpdirInstance( + (root) => + Effect.gen(function* () { + const svc = yield* Worktree.Service + const name = `remove-retry-${Date.now().toString(36)}` + const branch = `opencode/${name}` + const dir = path.join(root, "..", name) + + yield* Effect.promise(() => $`git worktree add --no-checkout -b ${branch} ${dir}`.cwd(root).quiet()) + yield* Effect.promise(() => $`git reset --hard`.cwd(dir).quiet()) + + const real = (yield* Effect.promise(() => $`which git`.quiet().text())).trim() + expect(real).toBeTruthy() + + const bin = path.join(root, "bin") + const shim = path.join(bin, "git") + const state = path.join(bin, "attempt") + yield* Effect.promise(() => fs.mkdir(bin, { recursive: true })) + yield* Effect.promise(() => + Bun.write( + shim, + [ + "#!/bin/bash", + `REAL_GIT=${JSON.stringify(real)}`, + `STATE=${JSON.stringify(state)}`, + 'if [ "$1" = "worktree" ] && [ "$2" = "remove" ] && [ ! -f "$STATE" ]; then', + ' touch "$STATE"', + ' echo "fatal: EBUSY: resource busy or locked, rmdir $4" >&2', + " exit 1", + "fi", + 'exec "$REAL_GIT" "$@"', + ].join("\n"), + ), + ) + yield* Effect.promise(() => fs.chmod(shim, 0o755)) + + const prev = yield* Effect.acquireRelease( + Effect.sync(() => { + const prev = process.env.PATH ?? "" + process.env.PATH = `${bin}${path.delimiter}${prev}` + return prev + }), + (prev) => + Effect.sync(() => { + process.env.PATH = prev + }), + ) + void prev + + const ok = yield* svc.remove({ directory: dir }) + + expect(ok).toBe(true) + expect( + yield* Effect.promise(() => + fs + .stat(dir) + .then(() => true) + .catch(() => false), + ), + ).toBe(false) + + const list = yield* Effect.promise(() => $`git worktree list --porcelain`.cwd(root).quiet().text()) + expect(list).not.toContain(`worktree ${dir}`) + }), + { git: true }, + ), + ) + // kilocode_change end + wintest("stops fsmonitor before removing a worktree", () => provideTmpdirInstance( (root) =>