diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a447ef0d..190358eb1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,10 @@ jobs: with: bun-version: "1.3.11" + # Restoring Bun's download cache on Windows was empirically slower than a cold install + # and consumed enough of the 30-minute job budget to starve opencode:test:ci. - uses: actions/cache@v4 + if: runner.os != 'Windows' with: path: | ~/.bun/install/cache diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 28370931f..1df65e49d 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -22,12 +22,11 @@ import { Instance, type InstanceContext } from "../project/instance" import { LSPServer } from "../lsp/server" import { Installation } from "@/installation" import { ConfigMarkdown } from "./markdown" -import { constants, existsSync } from "fs" +import { existsSync } from "fs" import { Bus } from "@/bus" import { GlobalBus } from "@/bus/global" import { Event } from "../server/event" import { Glob } from "../util/glob" -import { iife } from "@/util/iife" import { Account } from "@/account" import { isRecord } from "@/util/record" import { ConfigPaths } from "./paths" @@ -36,7 +35,7 @@ import type { ConsoleState } from "./console-state" import { AppFileSystem } from "@/filesystem" import { InstanceState } from "@/effect/instance-state" import { makeRuntime } from "@/effect/run-service" -import { Duration, Effect, Layer, Option, Context } from "effect" +import { Duration, Effect, Layer, Option, Context, Exit, Fiber } from "effect" import { Flock } from "@/util/flock" import { isPathPluginSpec, parsePluginSpecifier, resolvePathPluginTarget } from "@/plugin/shared" import { Npm } from "@/npm" @@ -142,6 +141,7 @@ export namespace Config { export type InstallInput = { signal?: AbortSignal waitTick?: (input: { dir: string; attempt: number; delay: number; waited: number }) => void | Promise + queueKey?: string | false } type Package = { @@ -149,58 +149,7 @@ export namespace Config { } export async function installDependencies(dir: string, input?: InstallInput) { - if (!(await isWritable(dir))) return - const key = process.platform === "win32" ? "config-install:win32" : `config-install:${Filesystem.resolve(dir)}` - await using _ = await Flock.acquire(key, { - signal: input?.signal, - onWait: (tick) => - input?.waitTick?.({ - dir, - attempt: tick.attempt, - delay: tick.delay, - waited: tick.waited, - }), - }) - input?.signal?.throwIfAborted() - - const pkg = path.join(dir, "package.json") - const plugin = path.join(dir, "node_modules", "@opencode-ai", "plugin", "package.json") - const target = Installation.isLocal() ? "*" : Installation.VERSION - const json = await Filesystem.readJson(pkg).catch( - (): Package => ({ - dependencies: {}, - }), - ) - const dependencies: Record = json.dependencies ?? {} - const hasDep = dependencies["@opencode-ai/plugin"] === target - json.dependencies = { - ...dependencies, - "@opencode-ai/plugin": target, - } - - const gitignore = path.join(dir, ".gitignore") - const ignore = await Filesystem.exists(gitignore) - const hasPkg = await Filesystem.exists(plugin) - if (!hasDep) { - await Filesystem.writeJson(pkg, json) - } - if (!ignore) { - await Filesystem.write( - gitignore, - ["node_modules", "package.json", "package-lock.json", "bun.lock", ".gitignore"].join("\n"), - ) - } - if (hasDep && ignore && hasPkg) return - await Npm.install(dir) - } - - async function isWritable(dir: string) { - try { - await fsNode.access(dir, constants.W_OK) - return true - } catch { - return false - } + return runPromise((svc) => svc.installDependencies(dir, input)) } function rel(item: string, patterns: string[]) { @@ -1125,7 +1074,10 @@ export namespace Config { type State = { config: Info directories: string[] - deps: Promise[] + deps: { + dir: string + fiber: Fiber.Fiber, never> + }[] consoleState: ConsoleState } @@ -1133,11 +1085,12 @@ export namespace Config { readonly get: () => Effect.Effect readonly getGlobal: () => Effect.Effect readonly getConsoleState: () => Effect.Effect + readonly installDependencies: (dir: string, input?: InstallInput) => Effect.Effect readonly update: (config: Info) => Effect.Effect readonly updateGlobal: (config: Info) => Effect.Effect readonly invalidate: (wait?: boolean) => Effect.Effect readonly directories: () => Effect.Effect - readonly waitForDependencies: () => Effect.Effect + readonly waitForDependencies: (directories?: string[]) => Effect.Effect } export class Service extends Context.Service()("@opencode/Config") {} @@ -1334,6 +1287,96 @@ export namespace Config { return yield* cachedGlobal }) + const install = Effect.fnUntraced(function* (dir: string) { + const pkg = path.join(dir, "package.json") + const plugin = path.join(dir, "node_modules", "@opencode-ai", "plugin", "package.json") + const target = Installation.isLocal() ? "*" : Installation.VERSION + const json = yield* fs.readJson(pkg).pipe( + Effect.catch(() => Effect.succeed({} satisfies Package)), + Effect.map((x): Package => (isRecord(x) ? (x as Package) : {})), + ) + const dependencies: Record = json.dependencies ?? {} + const hasDep = dependencies["@opencode-ai/plugin"] === target + const gitignore = path.join(dir, ".gitignore") + const ignore = yield* fs.existsSafe(gitignore) + const hasPkg = yield* fs.existsSafe(plugin) + if (!hasDep) { + yield* fs.writeJson(pkg, { + ...json, + dependencies: { + ...dependencies, + "@opencode-ai/plugin": target, + }, + }) + } + if (!ignore) { + yield* fs.writeFileString( + gitignore, + ["node_modules", "package.json", "package-lock.json", "bun.lock", ".gitignore"].join("\n"), + ) + } + if (hasDep && ignore && hasPkg) return + yield* Effect.promise(() => Npm.install(dir)) + }) + + const installDependencies = (dir: string, input?: InstallInput) => + Effect.gen(function* () { + const writable = yield* fs.access(dir, { writable: true }).pipe( + Effect.as(true), + Effect.catch(() => Effect.succeed(false)), + ) + if (!writable) return + + const controller = new AbortController() + const onAbort = () => controller.abort(input?.signal?.reason) + const scopedKey = `config-install:${Filesystem.resolve(dir)}` + const queueKey = input?.queueKey ?? (process.platform === "win32" ? "config-install:win32" : false) + const signal = (inner: AbortSignal) => AbortSignal.any([controller.signal, inner]) + const acquire = (key: string, waitTick = false) => + Effect.acquireRelease( + Effect.tryPromise({ + try: (inner) => + Flock.acquire(key, { + signal: signal(inner), + onWait: waitTick + ? (tick) => + input?.waitTick?.({ + dir, + attempt: tick.attempt, + delay: tick.delay, + waited: tick.waited, + }) + : undefined, + }), + catch: (cause) => cause, + }), + (lease) => Effect.promise(() => lease.release()), + { interruptible: true }, + ) + yield* Effect.acquireUseRelease( + Effect.sync(() => { + if (!input?.signal) return + if (input.signal.aborted) controller.abort(input.signal.reason) + else input.signal.addEventListener("abort", onAbort, { once: true }) + }), + () => + Effect.scoped( + Effect.gen(function* () { + if (queueKey) { + yield* acquire(queueKey, true) + } + yield* acquire(scopedKey, !queueKey) + input?.signal?.throwIfAborted() + yield* install(dir) + }), + ), + () => + Effect.sync(() => { + input?.signal?.removeEventListener("abort", onAbort) + }), + ) + }) + const loadInstanceState = Effect.fnUntraced(function* (ctx: InstanceContext) { const auth = yield* authSvc.all().pipe(Effect.orDie) @@ -1416,7 +1459,11 @@ export namespace Config { log.debug("loading config from OPENCODE_CONFIG_DIR", { path: Flag.OPENCODE_CONFIG_DIR }) } - const deps: Promise[] = [] + const deps: State["deps"] = [] + const queueKey = (dir: string) => { + if (process.platform !== "win32") return false + return dir.endsWith(".opencode") ? "config-install:win32:local" : "config-install:win32" + } for (const dir of unique(directories)) { if (dir.endsWith(".opencode") || dir === Flag.OPENCODE_CONFIG_DIR) { @@ -1430,13 +1477,18 @@ export namespace Config { } } - const dep = iife(async () => { - await installDependencies(dir) - }) - void dep.catch((err) => { - log.warn("background dependency install failed", { dir, error: err }) - }) - deps.push(dep) + const dep = yield* installDependencies(dir, { queueKey: queueKey(dir) }).pipe( + Effect.exit, + Effect.tap((exit) => + Exit.isFailure(exit) + ? Effect.sync(() => { + log.warn("background dependency install failed", { dir, error: String(exit.cause) }) + }) + : Effect.void, + ), + Effect.forkScoped, + ) + deps.push({ dir, fiber: dep }) result.command = mergeDeep(result.command ?? {}, yield* Effect.promise(() => loadCommand(dir))) result.agent = mergeDeep(result.agent, yield* Effect.promise(() => loadAgent(dir))) @@ -1571,8 +1623,17 @@ export namespace Config { return yield* InstanceState.use(state, (s) => s.consoleState) }) - const waitForDependencies = Effect.fn("Config.waitForDependencies")(function* () { - yield* InstanceState.useEffect(state, (s) => Effect.promise(() => Promise.all(s.deps).then(() => undefined))) + const waitForDependencies = Effect.fn("Config.waitForDependencies")(function* (directories?: string[]) { + const filter = directories ? new Set(directories.map((dir) => Filesystem.resolve(dir))) : undefined + yield* InstanceState.useEffect(state, (s) => + Effect.forEach( + s.deps.filter((dep) => !filter || filter.has(Filesystem.resolve(dep.dir))), + ({ fiber }) => Fiber.join(fiber), + { concurrency: "unbounded" }, + ).pipe( + Effect.flatMap((exits) => Effect.forEach(exits, (exit) => exit, { discard: true })), + ), + ) }) const update = Effect.fn("Config.update")(function* (config: Info) { @@ -1627,6 +1688,7 @@ export namespace Config { get, getGlobal, getConsoleState, + installDependencies, update, updateGlobal, invalidate, @@ -1672,7 +1734,7 @@ export namespace Config { return runPromise((svc) => svc.directories()) } - export async function waitForDependencies() { - return runPromise((svc) => svc.waitForDependencies()) + export async function waitForDependencies(directories?: string[]) { + return runPromise((svc) => svc.waitForDependencies(directories)) } } diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index 37c8d50c6..069cc8744 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -191,7 +191,7 @@ export namespace ToolRegistry { ) const cfg = yield* config.get() const rules = Permission.fromConfig(cfg.permission ?? {}) - let depsReady = false + const readyDirs = new Set() for (const match of matches) { const namespace = path.basename(match, path.extname(match)) const text = yield* Effect.promise(() => Bun.file(match).text()) @@ -206,9 +206,10 @@ export namespace ToolRegistry { ]) if (ids.length && ids.every((id) => disabled.has(id))) continue const spec = process.platform === "win32" ? match : pathToFileURL(match).href - if (!depsReady && (yield* Effect.promise(() => needsConfigDependencies(text, path.dirname(path.dirname(match)))))) { - depsReady = true - yield* config.waitForDependencies() + const toolDir = path.dirname(path.dirname(match)) + if (!readyDirs.has(toolDir) && (yield* Effect.promise(() => needsConfigDependencies(text, toolDir)))) { + readyDirs.add(toolDir) + yield* config.waitForDependencies([toolDir]).pipe(Effect.orDie) } const mod = yield* Effect.promise(() => import(spec)) for (const [id, def] of Object.entries(mod)) { diff --git a/packages/opencode/src/util/flock.ts b/packages/opencode/src/util/flock.ts index 74c7905eb..39c83d702 100644 --- a/packages/opencode/src/util/flock.ts +++ b/packages/opencode/src/util/flock.ts @@ -86,6 +86,30 @@ export namespace Flock { }) } + function retryableReleaseError(err: unknown) { + const errCode = code(err) + return errCode === "EBUSY" || errCode === "ENOTEMPTY" || errCode === "EPERM" + } + + async function removeLockDir(lockDir: string) { + let delay = 10 + + for (let attempt = 0; ; attempt += 1) { + try { + await rm(lockDir, { recursive: true, force: true }) + return + } catch (err) { + if (!retryableReleaseError(err) || attempt >= 4) { + throw err + } + } + + // Windows can briefly report the lock dir as still in use after heartbeat cleanup. + await sleep(delay) + delay *= 2 + } + } + function jitter(ms: number) { const j = Math.floor(ms * 0.3) const d = Math.floor(Math.random() * (2 * j + 1)) - j @@ -247,7 +271,7 @@ export namespace Flock { throw new Error("Refusing to release: lock token mismatch (not the owner).") } - await rm(lockDir, { recursive: true, force: true }) + await removeLockDir(lockDir) } return { @@ -314,9 +338,34 @@ export namespace Flock { }, cfg, ) + let abortReason: unknown + const onAbort = () => { + abortReason = input.signal?.reason ?? new Error("Aborted") + } + let released: Promise | undefined + const release = () => { + input.signal?.removeEventListener("abort", onAbort) + return (released ??= lock.release()) + } + + input.signal?.addEventListener("abort", onAbort, { once: true }) + if (input.signal?.aborted) { + abortReason = input.signal.reason ?? new Error("Aborted") + } + lock.startHeartbeat() - const release = () => lock.release() + // Hold the abort hook through the handoff microtask so callers can + // register their own finalizer without leaking a just-acquired lock. + if (input.signal) { + await Promise.resolve() + if (abortReason !== undefined || input.signal.aborted) { + await release() + throw (abortReason ?? input.signal.reason ?? new Error("Aborted")) + } + input.signal.removeEventListener("abort", onAbort) + } + return { release, [Symbol.asyncDispose]() { diff --git a/packages/opencode/src/worktree/index.ts b/packages/opencode/src/worktree/index.ts index f4ec0af83..ac9a2af1e 100644 --- a/packages/opencode/src/worktree/index.ts +++ b/packages/opencode/src/worktree/index.ts @@ -152,6 +152,18 @@ export namespace Worktree { ) } + function removeErrorCode(error: unknown) { + if (typeof error !== "object" || error === null || !("code" in error)) return + const value = error.code + if (typeof value !== "string") return + return value + } + + function retryableRemoveError(error: unknown) { + const code = removeErrorCode(error) + return code === "EBUSY" || code === "ENOTEMPTY" || code === "EPERM" + } + // --------------------------------------------------------------------------- // Effect service // --------------------------------------------------------------------------- @@ -358,14 +370,25 @@ export namespace Worktree { } function cleanDirectory(target: string) { - return Effect.promise(() => - import("fs/promises") - .then((fsp) => fsp.rm(target, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 })) - .catch((error) => { + return Effect.promise(async () => { + const fsp = await import("fs/promises") + let delay = 50 + + for (let attempt = 0; ; attempt += 1) { + try { + await fsp.rm(target, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }) + return + } catch (error) { + if (retryableRemoveError(error) && attempt < 7) { + await new Promise((resolve) => setTimeout(resolve, delay)) + delay = Math.min(delay * 2, 1_000) + continue + } const message = errorMessage(error) throw new RemoveFailedError({ message: message || "Failed to remove git worktree directory" }) - }), - ) + } + } + }) } const remove = Effect.fn("Worktree.remove")(function* (input: RemoveInput) { diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index 398d811d8..53c9664bf 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -24,6 +24,7 @@ import * as Network from "../../src/util/network" import { Npm } from "../../src/npm" import { writeMockConfigInstall } from "../shared/mock-npm-install" import { Installation } from "../../src/installation" +import { Flock } from "../../src/util/flock" const emptyAccount = Layer.mock(Account.Service)({ active: () => Effect.succeed(Option.none()), @@ -763,9 +764,16 @@ test("does not try to install dependencies in read-only OPENCODE_CONFIG_DIR", as }) const prev = process.env.OPENCODE_CONFIG_DIR - process.env.OPENCODE_CONFIG_DIR = tmp.extra + const prevGlobal = Global.Path.config + let globalDir = "" try { + globalDir = path.join(tmp.path, "global") + await fs.mkdir(globalDir, { recursive: true }) + process.env.OPENCODE_CONFIG_DIR = tmp.extra + ;(Global.Path as { config: string }).config = globalDir + await Config.invalidate() + await Instance.provide({ directory: tmp.path, fn: async () => { @@ -775,6 +783,8 @@ test("does not try to install dependencies in read-only OPENCODE_CONFIG_DIR", as } finally { if (prev === undefined) delete process.env.OPENCODE_CONFIG_DIR else process.env.OPENCODE_CONFIG_DIR = prev + ;(Global.Path as { config: string }).config = prevGlobal + await Config.invalidate() } }) @@ -788,11 +798,18 @@ test("installs dependencies in writable OPENCODE_CONFIG_DIR", async () => { }) const prev = process.env.OPENCODE_CONFIG_DIR - process.env.OPENCODE_CONFIG_DIR = tmp.extra + const prevGlobal = Global.Path.config + let globalDir = "" const online = spyOn(Network, "online").mockReturnValue(false) const install = spyOn(Npm, "install").mockImplementation((dir: string) => writeMockConfigInstall(dir)) try { + globalDir = path.join(tmp.path, "global") + await fs.mkdir(globalDir, { recursive: true }) + process.env.OPENCODE_CONFIG_DIR = tmp.extra + ;(Global.Path as { config: string }).config = globalDir + await Config.invalidate() + await Instance.provide({ directory: tmp.path, fn: async () => { @@ -809,6 +826,8 @@ test("installs dependencies in writable OPENCODE_CONFIG_DIR", async () => { install.mockRestore() if (prev === undefined) delete process.env.OPENCODE_CONFIG_DIR else process.env.OPENCODE_CONFIG_DIR = prev + ;(Global.Path as { config: string }).config = prevGlobal + await Config.invalidate() } }) @@ -953,6 +972,241 @@ test("skips reinstall when config dependencies are already bootstrapped", async } }) +test("Instance.disposeAll interrupts Config.get background installs for global config on Windows", async () => { + await using tmp = await tmpdir({ git: true }) + + const prevGlobal = Global.Path.config + let globalDir = "" + + const platform = process.platform + Object.defineProperty(process, "platform", { + value: "win32", + configurable: true, + }) + + let release = () => {} + let started = () => {} + const installing = new Promise((resolve) => { + release = resolve + }) + const ready = new Promise((resolve) => { + started = resolve + }) + const online = spyOn(Network, "online").mockReturnValue(false) + const install = spyOn(Npm, "install").mockImplementation(async (dir: string) => { + if (path.normalize(dir) === path.normalize(globalDir)) { + started() + await installing + } + await writeMockConfigInstall(dir) + }) + + try { + globalDir = path.join(tmp.path, "global") + await fs.mkdir(globalDir, { recursive: true }) + ;(Global.Path as { config: string }).config = globalDir + await Config.invalidate() + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await Config.get() + }, + }) + await ready + + const dispose = Promise.race([ + Instance.disposeAll().then(() => "done" as const), + Bun.sleep(200).then(() => "timeout" as const), + ]) + + expect(await dispose).toBe("done") + } finally { + release() + Object.defineProperty(process, "platform", { + value: platform, + configurable: true, + }) + online.mockRestore() + install.mockRestore() + ;(Global.Path as { config: string }).config = prevGlobal + await Config.invalidate() + } +}) + +test("Instance.disposeAll interrupts waiting background config installs without leaking Windows lock", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const cfg = path.join(dir, "configdir") + await fs.mkdir(cfg, { recursive: true }) + return cfg + }, + }) + + const prev = process.env.OPENCODE_CONFIG_DIR + const prevGlobal = Global.Path.config + const secondDir = path.join(tmp.path, "other-config") + let globalDir = "" + + const platform = process.platform + Object.defineProperty(process, "platform", { + value: "win32", + configurable: true, + }) + + let release = () => {} + let started = () => {} + let waiting = () => {} + const installing = new Promise((resolve) => { + release = resolve + }) + const ready = new Promise((resolve) => { + started = resolve + }) + const waitStarted = new Promise((resolve) => { + waiting = resolve + }) + const online = spyOn(Network, "online").mockReturnValue(false) + let secondCalls = 0 + const originalAcquire = Flock.acquire + let configAcquireCalls = 0 + const acquire = spyOn(Flock, "acquire").mockImplementation(async (key, input = {}) => { + if (key === "config-install:win32") { + configAcquireCalls += 1 + if (configAcquireCalls > 1) { + const onWait = input.onWait + input = { + ...input, + onWait: async (tick) => { + waiting() + await onWait?.(tick) + }, + } + } + } + return originalAcquire(key, input) + }) + const install = spyOn(Npm, "install").mockImplementation(async (dir: string) => { + if (path.normalize(dir) === path.normalize(tmp.extra)) { + started() + await installing + } + if (path.normalize(dir) === path.normalize(secondDir)) { + secondCalls += 1 + } + await writeMockConfigInstall(dir) + }) + + try { + globalDir = path.join(tmp.path, "global") + process.env.OPENCODE_CONFIG_DIR = secondDir + await fs.mkdir(globalDir, { recursive: true }) + ;(Global.Path as { config: string }).config = globalDir + await Config.invalidate() + await fs.mkdir(secondDir, { recursive: true }) + + const first = Config.installDependencies(tmp.extra) + await ready + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await Config.get() + }, + }) + await waitStarted + const dispose = Promise.race([ + Instance.disposeAll().then(() => "done" as const), + Bun.sleep(200).then(() => "timeout" as const), + ]) + + expect(await dispose).toBe("done") + + release() + await first + await Bun.sleep(50) + expect(secondCalls).toBe(0) + + await Config.installDependencies(secondDir) + expect(secondCalls).toBe(1) + } finally { + release() + Object.defineProperty(process, "platform", { + value: platform, + configurable: true, + }) + online.mockRestore() + acquire.mockRestore() + install.mockRestore() + if (prev === undefined) delete process.env.OPENCODE_CONFIG_DIR + else process.env.OPENCODE_CONFIG_DIR = prev + ;(Global.Path as { config: string }).config = prevGlobal + await Config.invalidate() + } +}) + +test("config dependency install aborts while waiting for Windows lock", async () => { + await using tmp = await tmpdir() + const firstDir = path.join(tmp.path, "a") + const secondDir = path.join(tmp.path, "b") + await fs.mkdir(firstDir, { recursive: true }) + await fs.mkdir(secondDir, { recursive: true }) + + const platform = process.platform + Object.defineProperty(process, "platform", { + value: "win32", + configurable: true, + }) + + let release = () => {} + let started = () => {} + const installing = new Promise((resolve) => { + release = resolve + }) + const ready = new Promise((resolve) => { + started = resolve + }) + const online = spyOn(Network, "online").mockReturnValue(false) + let secondCalls = 0 + const install = spyOn(Npm, "install").mockImplementation(async (dir: string) => { + if (path.normalize(dir) === path.normalize(firstDir)) { + started() + await installing + } + if (path.normalize(dir) === path.normalize(secondDir)) { + secondCalls += 1 + } + await writeMockConfigInstall(dir) + }) + + try { + const first = Config.installDependencies(firstDir) + await ready + + const controller = new AbortController() + await expect( + Config.installDependencies(secondDir, { + signal: controller.signal, + waitTick: () => { + controller.abort(new Error("stop waiting")) + }, + }), + ).rejects.toThrow("stop waiting") + + release() + await first + expect(secondCalls).toBe(0) + } finally { + release() + Object.defineProperty(process, "platform", { + value: platform, + configurable: true, + }) + online.mockRestore() + install.mockRestore() + } +}) + test("resolves scoped npm plugins in config", async () => { await using tmp = await tmpdir({ init: async (dir) => { diff --git a/packages/opencode/test/config/tui.test.ts b/packages/opencode/test/config/tui.test.ts index b761d59ea..8661abb41 100644 --- a/packages/opencode/test/config/tui.test.ts +++ b/packages/opencode/test/config/tui.test.ts @@ -1,16 +1,39 @@ -import { afterEach, beforeEach, expect, test } from "bun:test" +import { afterEach, beforeEach, expect, spyOn, test } from "bun:test" import path from "path" import fs from "fs/promises" -import { tmpdir } from "../fixture/fixture" +import { tmpdir as baseTmpdir } from "../fixture/fixture" import { Instance } from "../../src/project/instance" import { Config } from "../../src/config/config" import { TuiConfig } from "../../src/config/tui" import { Global } from "../../src/global" +import { Npm } from "../../src/npm" import { Filesystem } from "../../src/util/filesystem" +import { writeMockConfigInstall } from "../shared/mock-npm-install" const managedConfigDir = process.env.OPENCODE_TEST_MANAGED_CONFIG_DIR! const wintest = process.platform === "win32" ? test : test.skip +type TestTmpdirOptions = { + git?: boolean + config?: Partial + init?: (dir: string) => Promise + dispose?: (dir: string) => Promise +} + +async function tmpdir(options?: TestTmpdirOptions) { + return baseTmpdir({ + ...options, + dispose: async (dir) => { + await Instance.provide({ + directory: dir, + fn: () => TuiConfig.waitForDependencies(), + }) + await Config.invalidate(true) + return (await options?.dispose?.(dir)) as T + }, + }) +} + beforeEach(async () => { await Config.invalidate(true) }) @@ -27,6 +50,8 @@ afterEach(async () => { }) test("keeps server and tui plugin merge semantics aligned", async () => { + const install = spyOn(Npm, "install").mockImplementation((dir: string) => writeMockConfigInstall(dir)) + await using tmp = await tmpdir({ init: async (dir) => { const local = path.join(dir, ".opencode") @@ -76,25 +101,31 @@ test("keeps server and tui plugin merge semantics aligned", async () => { }, }) - await Instance.provide({ - directory: tmp.path, - fn: async () => { - const server = await Config.get() - const tui = await TuiConfig.get() - const serverPlugins = (server.plugin ?? []).map((item) => Config.pluginSpecifier(item)) - const tuiPlugins = (tui.plugin ?? []).map((item) => Config.pluginSpecifier(item)) - - expect(serverPlugins).toEqual(tuiPlugins) - expect(serverPlugins).toContain("shared-plugin@2.0.0") - expect(serverPlugins).not.toContain("shared-plugin@1.0.0") - - const serverOrigins = server.plugin_origins ?? [] - const tuiOrigins = tui.plugin_origins ?? [] - expect(serverOrigins.map((item) => Config.pluginSpecifier(item.spec))).toEqual(serverPlugins) - expect(tuiOrigins.map((item) => Config.pluginSpecifier(item.spec))).toEqual(tuiPlugins) - expect(serverOrigins.map((item) => item.scope)).toEqual(tuiOrigins.map((item) => item.scope)) - }, - }) + try { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const server = await Config.get() + await Config.waitForDependencies() + const tui = await TuiConfig.get() + const serverPlugins = (server.plugin ?? []).map((item) => Config.pluginSpecifier(item)) + const tuiPlugins = (tui.plugin ?? []).map((item) => Config.pluginSpecifier(item)) + + expect(serverPlugins).toEqual(tuiPlugins) + expect(serverPlugins).toContain("shared-plugin@2.0.0") + expect(serverPlugins).not.toContain("shared-plugin@1.0.0") + + const serverOrigins = server.plugin_origins ?? [] + const tuiOrigins = tui.plugin_origins ?? [] + expect(serverOrigins.map((item) => Config.pluginSpecifier(item.spec))).toEqual(serverPlugins) + expect(tuiOrigins.map((item) => Config.pluginSpecifier(item.spec))).toEqual(tuiPlugins) + expect(serverOrigins.map((item) => item.scope)).toEqual(tuiOrigins.map((item) => item.scope)) + expect(install).toHaveBeenCalledWith(path.join(tmp.path, ".opencode")) + }, + }) + } finally { + install.mockRestore() + } }) test("loads tui config with the same precedence order as server config paths", async () => { diff --git a/packages/opencode/test/project/worktree-cleanup.test.ts b/packages/opencode/test/project/worktree-cleanup.test.ts new file mode 100644 index 000000000..1e46cf8c8 --- /dev/null +++ b/packages/opencode/test/project/worktree-cleanup.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, mock, test } from "bun:test" +import { $ } from "bun" +import { promises as nodeFs } from "node:fs" +import path from "path" +import { Instance } from "../../src/project/instance" +import { Filesystem } from "../../src/util/filesystem" +import { tmpdir } from "../fixture/fixture" + +let rmCalls = 0 +let failBusyTimes = 0 + +mock.module("fs/promises", () => { + return { + ...nodeFs, + rm: async (...args: Parameters) => { + rmCalls += 1 + if (failBusyTimes > 0) { + failBusyTimes -= 1 + const error = new Error("resource busy or locked") + ;(error as NodeJS.ErrnoException).code = "EBUSY" + throw error + } + return nodeFs.rm(...args) + }, + } +}) + +const { Worktree } = await import("../../src/worktree") + +describe("Worktree.remove cleanup", () => { + test("retries transient busy directory cleanup after git worktree removal", async () => { + await using tmp = await tmpdir({ git: true }) + const root = tmp.path + const name = `remove-busy-${Date.now().toString(36)}` + const branch = `opencode/${name}` + const dir = path.join(root, "..", name) + + await $`git worktree add --no-checkout -b ${branch} ${dir}`.cwd(root).quiet() + await $`git reset --hard`.cwd(dir).quiet() + + failBusyTimes = 2 + rmCalls = 0 + + const ok = await Instance.provide({ + directory: root, + fn: () => Worktree.remove({ directory: dir }), + }) + + expect(ok).toBe(true) + expect(rmCalls).toBeGreaterThanOrEqual(3) + expect(await Filesystem.exists(dir)).toBe(false) + }) +}) diff --git a/packages/opencode/test/tool/registry.test.ts b/packages/opencode/test/tool/registry.test.ts index 0fd341380..11403c896 100644 --- a/packages/opencode/test/tool/registry.test.ts +++ b/packages/opencode/test/tool/registry.test.ts @@ -6,11 +6,30 @@ import { writeMockConfigInstall } from "../shared/mock-npm-install" import { Instance } from "../../src/project/instance" import { ToolRegistry } from "../../src/tool/registry" import { Npm } from "../../src/npm" +import { Config } from "../../src/config/config" +import { Global } from "../../src/global" +import { withTimeout } from "../../src/util/timeout" afterEach(async () => { await Instance.disposeAll() }) +async function withPlatform(value: NodeJS.Platform, fn: () => Promise) { + const previous = process.platform + Object.defineProperty(process, "platform", { + value, + configurable: true, + }) + try { + return await fn() + } finally { + Object.defineProperty(process, "platform", { + value: previous, + configurable: true, + }) + } +} + describe("tool.registry", () => { test("loads tools from .opencode/tool (singular)", async () => { await using tmp = await tmpdir({ @@ -196,6 +215,220 @@ describe("tool.registry", () => { } }) + test("does not wait for unrelated global config installs before importing local tools with bare imports", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const toolsDir = path.join(dir, ".opencode", "tools") + await fs.mkdir(toolsDir, { recursive: true }) + + await Bun.write( + path.join(toolsDir, "late.ts"), + [ + "import { ready } from 'late-dep'", + "export default {", + " description: 'tool that only needs local config deps',", + " args: {},", + " execute: async () => ready,", + "}", + "", + ].join("\n"), + ) + }, + }) + + const prevGlobal = Global.Path.config + let globalDir = "" + let release = () => {} + let started = () => {} + let idsTask: Promise | undefined + const installing = new Promise((resolve) => { + release = resolve + }) + const ready = new Promise((resolve) => { + started = resolve + }) + const install = spyOn(Npm, "install").mockImplementation(async (dir: string) => { + if (path.normalize(dir) === path.normalize(globalDir)) { + started() + await installing + } + await writeMockConfigInstall(dir) + }) + + try { + globalDir = path.join(tmp.path, "global") + await fs.mkdir(globalDir, { recursive: true }) + ;(Global.Path as { config: string }).config = globalDir + await Config.invalidate(true) + + idsTask = Instance.provide({ + directory: tmp.path, + fn: () => ToolRegistry.ids(), + }).then((ids) => ids) + + await ready + + await expect(withTimeout(idsTask, 2_000)).resolves.toContain("late") + } finally { + release() + await idsTask?.catch(() => undefined) + install.mockRestore() + ;(Global.Path as { config: string }).config = prevGlobal + await Config.invalidate(true) + } + }) + + test("does not wait for unrelated global config installs on Windows before importing local tools with bare imports", async () => { + await withPlatform("win32", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const toolsDir = path.join(dir, ".opencode", "tools") + await fs.mkdir(toolsDir, { recursive: true }) + + await Bun.write( + path.join(toolsDir, "late.ts"), + [ + "import { ready } from 'late-dep'", + "export default {", + " description: 'tool that only needs local config deps',", + " args: {},", + " execute: async () => ready,", + "}", + "", + ].join("\n"), + ) + }, + }) + + const prevGlobal = Global.Path.config + let globalDir = "" + let release = () => {} + let started = () => {} + let idsTask: Promise | undefined + const installing = new Promise((resolve) => { + release = resolve + }) + const ready = new Promise((resolve) => { + started = resolve + }) + const install = spyOn(Npm, "install").mockImplementation(async (dir: string) => { + if (path.normalize(dir) === path.normalize(globalDir)) { + started() + await installing + } + await writeMockConfigInstall(dir) + }) + + try { + globalDir = path.join(tmp.path, "global") + await fs.mkdir(globalDir, { recursive: true }) + ;(Global.Path as { config: string }).config = globalDir + await Config.invalidate(true) + + idsTask = Instance.provide({ + directory: tmp.path, + fn: () => ToolRegistry.ids(), + }).then((ids) => ids) + + await ready + + await expect(withTimeout(idsTask, 2_000)).resolves.toContain("late") + } finally { + release() + await idsTask?.catch(() => undefined) + install.mockRestore() + ;(Global.Path as { config: string }).config = prevGlobal + await Config.invalidate(true) + } + }) + }) + + test("serializes concurrent Windows local tool dependency installs across directories", async () => { + await withPlatform("win32", async () => { + async function createToolProject() { + return await tmpdir({ + init: async (dir) => { + const toolsDir = path.join(dir, ".opencode", "tools") + await fs.mkdir(toolsDir, { recursive: true }) + await Bun.write( + path.join(toolsDir, "late.ts"), + [ + "import { ready } from 'late-dep'", + "export default {", + " description: 'tool that needs local config deps',", + " args: {},", + " execute: async () => ready,", + "}", + "", + ].join("\n"), + ) + }, + }) + } + + await using first = await createToolProject() + await using second = await createToolProject() + + const targets = new Set([ + path.normalize(path.join(first.path, ".opencode")), + path.normalize(path.join(second.path, ".opencode")), + ]) + let open = 0 + let peak = 0 + let calls = 0 + let release = () => {} + let started = () => {} + const gate = new Promise((resolve) => { + release = resolve + }) + const firstInstall = new Promise((resolve) => { + started = resolve + }) + const install = spyOn(Npm, "install").mockImplementation(async (dir: string) => { + const key = path.normalize(dir) + const hit = targets.has(key) + if (hit) { + calls += 1 + open += 1 + peak = Math.max(peak, open) + if (calls === 1) { + started() + await gate + } + } + await writeMockConfigInstall(dir) + if (hit) { + open -= 1 + } + }) + + try { + const firstIds = Instance.provide({ + directory: first.path, + fn: () => ToolRegistry.ids(), + }) + await firstInstall + + const secondIds = Instance.provide({ + directory: second.path, + fn: () => ToolRegistry.ids(), + }) + await Bun.sleep(100) + release() + + await expect(firstIds).resolves.toContain("late") + await expect(secondIds).resolves.toContain("late") + } finally { + release() + install.mockRestore() + await Config.invalidate(true) + } + + expect(calls).toBe(2) + expect(peak).toBe(1) + }) + }) + test("skips disabled tools before importing them", async () => { await using tmp = await tmpdir({ init: async (dir) => { diff --git a/packages/opencode/test/util/flock-release-retry.test.ts b/packages/opencode/test/util/flock-release-retry.test.ts new file mode 100644 index 000000000..60bd48dbd --- /dev/null +++ b/packages/opencode/test/util/flock-release-retry.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, test } from "bun:test" +import fs from "fs/promises" +import path from "path" +import { pathToFileURL } from "url" +import { tmpdir } from "../fixture/fixture" + +async function importFlockWithTransientReleaseFailure(root: string, tempDir: string) { + const source = path.join(root, "src", "util", "flock.ts") + await fs.mkdir(tempDir, { recursive: true }) + + const mockFsPath = path.join(tempDir, "mock-fs-promises.ts") + const flockPath = path.join(tempDir, "flock-under-test.ts") + const original = await fs.readFile(source, "utf8") + const globalUrl = pathToFileURL(path.join(root, "src", "global", "index.ts")).href + const hashUrl = pathToFileURL(path.join(root, "src", "util", "hash.ts")).href + + await fs.writeFile( + mockFsPath, + [ + 'import * as real from "fs/promises"', + 'export const mkdir = real.mkdir', + 'export const readFile = real.readFile', + 'export const stat = real.stat', + 'export const utimes = real.utimes', + 'export const writeFile = real.writeFile', + "", + "let failed = false", + "", + "export async function rm(target: Parameters[0], options?: Parameters[1]) {", + ' if (!failed && typeof target === "string" && target.endsWith(".lock")) {', + " failed = true", + ' const error = new Error("transient lock-dir removal failure") as Error & { code?: string }', + ' error.code = "ENOTEMPTY"', + " throw error", + " }", + " return real.rm(target, options)", + "}", + "", + ].join("\n"), + "utf8", + ) + + await fs.writeFile( + flockPath, + original + .replace('from "fs/promises"', 'from "./mock-fs-promises.ts"') + .replace('from "@/global"', `from "${globalUrl}"`) + .replace('from "@/util/hash"', `from "${hashUrl}"`), + "utf8", + ) + + return import(pathToFileURL(flockPath).href + `?t=${Date.now()}`) +} + +describe("util.flock release retry", () => { + test("retries transient lock-dir removal failures during release", async () => { + await using tmp = await tmpdir() + const root = path.join(import.meta.dir, "../..") + const { Flock } = await importFlockWithTransientReleaseFailure(root, path.join(tmp.path, "module-fixtures")) + const dir = path.join(tmp.path, "locks") + const key = "flock:release-retry" + + const lease = await Flock.acquire(key, { + dir, + staleMs: 1_000, + timeoutMs: 1_000, + baseDelayMs: 10, + maxDelayMs: 20, + }) + + await expect(lease.release()).resolves.toBeUndefined() + + let reacquired = false + await Flock.withLock( + key, + async () => { + reacquired = true + }, + { + dir, + staleMs: 1_000, + timeoutMs: 1_000, + baseDelayMs: 10, + maxDelayMs: 20, + }, + ) + + expect(reacquired).toBe(true) + }) +}) diff --git a/packages/opencode/test/util/flock.test.ts b/packages/opencode/test/util/flock.test.ts index fedbfb069..9f212f773 100644 --- a/packages/opencode/test/util/flock.test.ts +++ b/packages/opencode/test/util/flock.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test" import fs from "fs/promises" import path from "path" +import { pathToFileURL } from "url" import { Flock } from "../../src/util/flock" import { Hash } from "../../src/util/hash" import { Process } from "../../src/util/process" @@ -66,6 +67,49 @@ function spawn(msg: Msg) { }) } +async function importFlockWithAfterAcquireHook(root: string, tempDir: string) { + const source = path.join(root, "src", "util", "flock.ts") + await fs.mkdir(tempDir, { recursive: true }) + + const hookPath = path.join(tempDir, "after-acquire.ts") + const flockPath = path.join(tempDir, "flock-under-test.ts") + const original = await fs.readFile(source, "utf8") + const globalUrl = pathToFileURL(path.join(root, "src", "global", "index.ts")).href + const hashUrl = pathToFileURL(path.join(root, "src", "util", "hash.ts")).href + + await fs.writeFile( + hookPath, + [ + "let hook: (() => Promise) | undefined", + "", + "export function setAfterAcquire(next: (() => Promise) | undefined) {", + " hook = next", + "}", + "", + "export function afterAcquire() {", + " return hook?.()", + "}", + "", + ].join("\n"), + "utf8", + ) + + await fs.writeFile( + flockPath, + original + .replace('import { Global } from "@/global"', `import { Global } from "${globalUrl}"`) + .replace('import { Hash } from "@/util/hash"', `import { Hash } from "${hashUrl}"\nimport { afterAcquire } from "./after-acquire.ts"`) + .replace(" lock.startHeartbeat()", " await afterAcquire()\n lock.startHeartbeat()") + .concat('\nexport { setAfterAcquire } from "./after-acquire.ts"\n'), + "utf8", + ) + + return import(pathToFileURL(flockPath).href + `?t=${Date.now()}`) as Promise<{ + Flock: typeof Flock + setAfterAcquire: (next: (() => Promise) | undefined) => void + }> +} + describe("util.flock", () => { test("enforces mutual exclusion under process contention", async () => { await using tmp = await tmpdir() @@ -314,6 +358,52 @@ describe("util.flock", () => { expect(await exists(lockDir)).toBe(false) }) + test("releases the lock when aborted immediately after acquisition", async () => { + await using tmp = await tmpdir() + const root = path.join(import.meta.dir, "../..") + const { Flock, setAfterAcquire } = await importFlockWithAfterAcquireHook(root, path.join(tmp.path, "module-fixtures")) + const dir = path.join(tmp.path, "locks") + const key = "flock:post-acquire-abort" + const lockDir = lock(dir, key) + const controller = new AbortController() + + setAfterAcquire(async () => { + controller.abort(new Error("stop")) + await Promise.resolve() + }) + + const err = await Flock.acquire(key, { + dir, + signal: controller.signal, + staleMs: 1_000, + timeoutMs: 1_000, + baseDelayMs: 10, + maxDelayMs: 20, + }).catch((err) => err) + + expect(err).toBeInstanceOf(Error) + if (!(err instanceof Error)) throw err + expect(err.message).toContain("stop") + expect(await exists(lockDir)).toBe(false) + + let reacquired = false + await Flock.withLock( + key, + async () => { + reacquired = true + }, + { + dir, + staleMs: 1_000, + timeoutMs: 1_000, + baseDelayMs: 10, + maxDelayMs: 20, + }, + ) + + expect(reacquired).toBe(true) + }) + test("refuses token mismatch release and recovers from stale", async () => { await using tmp = await tmpdir() const dir = path.join(tmp.path, "locks")