diff --git a/packages/opencode/src/kilocode/worktree-cleanup.ts b/packages/opencode/src/kilocode/worktree-cleanup.ts new file mode 100644 index 00000000000..126dac8d8f1 --- /dev/null +++ b/packages/opencode/src/kilocode/worktree-cleanup.ts @@ -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 => + 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(input: { + root: string + target: string + git: (args: string[], opts?: { cwd?: string }) => Effect.Effect + stop: (target: string) => Effect.Effect + }) { + 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 + }) + } +} diff --git a/packages/opencode/src/worktree/index.ts b/packages/opencode/src/worktree/index.ts index 47b5dbe124f..c3bd4eac248 100644 --- a/packages/opencode/src/worktree/index.ts +++ b/packages/opencode/src/worktree/index.ts @@ -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 => - 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) { diff --git a/packages/opencode/test/fixture/fixture.ts b/packages/opencode/test/fixture/fixture.ts index cbfceaa535c..51e1409a847 100644 --- a/packages/opencode/test/fixture/fixture.ts +++ b/packages/opencode/test/fixture/fixture.ts @@ -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 => { - 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 diff --git a/packages/opencode/test/kilocode/cleanup.ts b/packages/opencode/test/kilocode/cleanup.ts new file mode 100644 index 00000000000..431cae59fcc --- /dev/null +++ b/packages/opencode/test/kilocode/cleanup.ts @@ -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 => { + 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) +} diff --git a/packages/opencode/test/kilocode/worktree-remove-lock.test.ts b/packages/opencode/test/kilocode/worktree-remove-lock.test.ts new file mode 100644 index 00000000000..e52eacd1568 --- /dev/null +++ b/packages/opencode/test/kilocode/worktree-remove-lock.test.ts @@ -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 }, + ), + ) +}) diff --git a/packages/opencode/test/preload.ts b/packages/opencode/test/preload.ts index 7ecfd6d2d84..678f048459e 100644 --- a/packages/opencode/test/preload.ts +++ b/packages/opencode/test/preload.ts @@ -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 => { - 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") diff --git a/packages/opencode/test/project/worktree-remove.test.ts b/packages/opencode/test/project/worktree-remove.test.ts index 703ec3a497f..5fb2beb2863 100644 --- a/packages/opencode/test/project/worktree-remove.test.ts +++ b/packages/opencode/test/project/worktree-remove.test.ts @@ -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) =>