Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/windows-worktree-cleanup.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@kilocode/cli": patch
---

Improve Windows worktree cleanup reliability when file handles are released slowly.
69 changes: 69 additions & 0 deletions packages/opencode/src/kilocode/worktree-cleanup.ts
Original file line number Diff line number Diff line change
@@ -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
})
}
}
16 changes: 8 additions & 8 deletions packages/opencode/src/worktree/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" })

Expand Down Expand Up @@ -353,18 +354,18 @@ export const layer: Layer.Layer<
)
}

// kilocode_change start - use Kilo cleanup helper for slow Windows handle release
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) => {
return Effect.promise(() => WorktreeCleanup.removeDirectory(target)).pipe(
Effect.catch((error) =>
Effect.sync(() => {
const message = errorMessage(error)
throw new RemoveFailedError({ message: message || "Failed to remove git worktree directory" })
}),
),
)
}
// kilocode_change end

const remove = Effect.fn("Worktree.remove")(function* (input: RemoveInput) {
const ctx = yield* InstanceState.context
Expand All @@ -391,8 +392,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* 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) {
Expand Down
8 changes: 2 additions & 6 deletions packages/opencode/test/fixture/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -24,12 +25,7 @@ function exists(dir: string) {
}

function clean(dir: string) {
return fs.rm(dir, {
recursive: true,
force: true,
maxRetries: 5,
retryDelay: 100,
})
return cleanup(dir) // kilocode_change
}

async function stop(dir: string) {
Expand Down
30 changes: 30 additions & 0 deletions packages/opencode/test/kilocode/cleanup.ts
Original file line number Diff line number Diff line change
@@ -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)
}
82 changes: 82 additions & 0 deletions packages/opencode/test/kilocode/worktree-remove-lock.test.ts
Original file line number Diff line number Diff line change
@@ -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 },
),
)
})
18 changes: 2 additions & 16 deletions packages/opencode/test/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,16 @@
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)
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"
const rm = async (left: number): Promise<void> => {
Bun.gc(true)
await sleep(100)
return fs.rm(dir, { recursive: true, force: true }).catch((error) => {
if (!busy(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)
await cleanup(dir) // kilocode_change
})

process.env["XDG_DATA_HOME"] = path.join(dir, "share")
Expand Down
Loading