refactor(cli): isolate Windows cleanup helpers

This commit is contained in:
Alex Alecu
2026-04-28 12:04:46 +03:00
parent 4145e48e82
commit d44f65576b
7 changed files with 188 additions and 179 deletions
@@ -0,0 +1,69 @@
import * as fs from "fs/promises"
import { Effect } from "effect"
type GitResult = { code: number; text: string; stderr: string }
function opts() {
return process.platform === "win32" ? { retries: 60, delay: 500 } : { retries: 5, delay: 100 }
}
function locked(error: unknown) {
return (
typeof error === "object" &&
error !== null &&
"code" in error &&
["EBUSY", "EACCES", "EPERM"].includes(String(error.code))
)
}
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))
}
export namespace WorktreeCleanup {
export async function removeDirectory(target: string) {
const cfg = opts()
const rm = async (left: number): Promise<void> =>
fs
.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(cfg.delay)
return rm(left - 1)
})
return rm(cfg.retries)
}
export function remove<R, E, R2, E2>(input: {
root: string
target: string
git: (args: string[], opts?: { cwd?: string }) => Effect.Effect<GitResult, E, R>
stop: (target: string) => Effect.Effect<unknown, E2, R2>
}) {
const cfg = opts()
return Effect.gen(function* () {
for (const attempt of Array.from({ length: cfg.retries }, (_, i) => i)) {
yield* input.stop(input.target)
const result = yield* input.git(["worktree", "remove", "--force", input.target], { cwd: input.root })
if (result.code === 0) return result
if (!transient(result)) return result
if (attempt === cfg.retries - 1) return result
if (process.platform === "win32") yield* Effect.sync(() => Bun.gc(true))
yield* Effect.sleep(`${cfg.delay} millis`)
}
return { code: 1, text: "", stderr: "Failed to remove git worktree" } satisfies GitResult
})
}
}
+3 -57
View File
@@ -20,6 +20,7 @@ import { AppFileSystem } from "@opencode-ai/shared/filesystem"
import { BootstrapRuntime } from "@/effect/bootstrap-runtime"
import * as CrossSpawnSpawner from "@/effect/cross-spawn-spawner"
import { InstanceState } from "@/effect"
import { WorktreeCleanup } from "@/kilocode/worktree-cleanup" // kilocode_change
const log = Log.create({ service: "worktree" })
@@ -353,33 +354,8 @@ 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" ? 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<void> =>
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(
return Effect.promise(() => WorktreeCleanup.removeDirectory(target)).pipe( // kilocode_change
Effect.catch((error) =>
Effect.sync(() => {
const message = errorMessage(error)
@@ -389,36 +365,6 @@ export const layer: Layer.Layer<
)
}
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") {
@@ -444,7 +390,7 @@ export const layer: Layer.Layer<
return true
}
const removed = yield* removeWorktree(ctx.worktree, entry.path) // kilocode_change
const removed = yield* WorktreeCleanup.remove({ root: ctx.worktree, target: entry.path, git, stop: stopFsmonitor }) // kilocode_change
if (removed.code !== 0) {
const next = yield* git(["worktree", "list", "--porcelain"], { cwd: ctx.worktree })
if (next.code !== 0) {
+2 -30
View File
@@ -10,6 +10,7 @@ import type { Config } from "../../src/config"
import { InstanceRef } from "../../src/effect/instance-ref"
import { Instance } from "../../src/project/instance"
import { TestLLMServer } from "../lib/llm-server"
import { remove as cleanup } from "../kilocode/cleanup" // kilocode_change
// Strip null bytes from paths (defensive fix for CI environment issues)
function sanitizePath(p: string): string {
@@ -23,38 +24,9 @@ function exists(dir: string) {
.catch(() => false)
}
// 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<void> => {
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)
return cleanup(dir) // kilocode_change
}
// kilocode_change end
async function stop(dir: string) {
if (!(await exists(dir))) return
@@ -0,0 +1,30 @@
import * as fs from "fs/promises"
function opts() {
return process.platform === "win32" ? { retries: 60, delay: 500 } : { retries: 5, delay: 100 }
}
function locked(error: unknown) {
return (
typeof error === "object" &&
error !== null &&
"code" in error &&
["EBUSY", "EACCES", "EPERM"].includes(String(error.code))
)
}
export async function remove(dir: string) {
const cfg = opts()
const rm = async (left: number): Promise<void> => {
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(cfg.delay)
return rm(left - 1)
})
}
return rm(cfg.retries)
}
@@ -0,0 +1,82 @@
import { $ } from "bun"
import { describe, expect } from "bun:test"
import * as fs from "fs/promises"
import path from "path"
import { Effect, Layer } from "effect"
import * as CrossSpawnSpawner from "@/effect/cross-spawn-spawner"
import { Worktree } from "../../src/worktree"
import { provideTmpdirInstance } from "../fixture/fixture"
import { testEffect } from "../lib/effect"
const it = testEffect(Layer.mergeAll(Worktree.defaultLayer, CrossSpawnSpawner.defaultLayer))
describe("Worktree.remove lock retries", () => {
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 },
),
)
})
+2 -21
View File
@@ -3,8 +3,8 @@
import os from "os"
import path from "path"
import fs from "fs/promises"
import { setTimeout as sleep } from "node:timers/promises"
import { afterAll } from "bun:test"
import { remove as cleanup } from "./kilocode/cleanup" // kilocode_change
// Set XDG env vars FIRST, before any src/ imports
const dir = path.join(os.tmpdir(), "opencode-test-data-" + process.pid)
@@ -12,26 +12,7 @@ await fs.mkdir(dir, { recursive: true })
afterAll(async () => {
const { Database } = await import("../src/storage")
Database.close()
// 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<void> => {
Bun.gc(true)
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 lock errors in test cleanup.
await rm(60)
// kilocode_change end
await cleanup(dir) // kilocode_change
})
process.env["XDG_DATA_HOME"] = path.join(dir, "share")
@@ -84,77 +84,6 @@ 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) =>