From ec9c9960a3f6a141f4ebb78028cf5e88970b0a96 Mon Sep 17 00:00:00 2001 From: Dax Raad Date: Fri, 27 Mar 2026 11:19:38 -0400 Subject: [PATCH] refactor: migrate from BunProc to Npm module - Replace BunProc.install with Npm.add for plugin resolution - Remove needsInstall check from installDependencies - Update test files to use Npm module instead of BunProc - Delete bun/index.ts and bun/registry.ts (no longer needed) --- packages/opencode/src/bun/index.ts | 128 ------------------ packages/opencode/src/bun/registry.ts | 50 ------- packages/opencode/src/config/config.ts | 94 +------------ packages/opencode/src/npm/index.ts | 3 +- packages/opencode/src/plugin/shared.ts | 4 +- .../cli/tui/plugin-loader-entrypoint.test.ts | 6 +- packages/opencode/test/config/config.test.ts | 43 ++---- .../test/plugin/loader-shared.test.ts | 24 ++-- 8 files changed, 38 insertions(+), 314 deletions(-) delete mode 100644 packages/opencode/src/bun/index.ts delete mode 100644 packages/opencode/src/bun/registry.ts diff --git a/packages/opencode/src/bun/index.ts b/packages/opencode/src/bun/index.ts deleted file mode 100644 index dbdf5a2bc4..0000000000 --- a/packages/opencode/src/bun/index.ts +++ /dev/null @@ -1,128 +0,0 @@ -import z from "zod" -import { Global } from "../global" -import { Log } from "../util/log" -import path from "path" -import { Filesystem } from "../util/filesystem" -import { NamedError } from "@opencode-ai/util/error" -import { Lock } from "../util/lock" -import { PackageRegistry } from "./registry" -import { online, proxied } from "@/util/network" -import { Process } from "../util/process" - -export namespace BunProc { - const log = Log.create({ service: "bun" }) - - export async function run(cmd: string[], options?: Process.RunOptions) { - const full = [which(), ...cmd] - log.info("running", { - cmd: full, - ...options, - }) - const result = await Process.run(full, { - cwd: options?.cwd, - abort: options?.abort, - kill: options?.kill, - timeout: options?.timeout, - nothrow: options?.nothrow, - env: { - ...process.env, - ...options?.env, - BUN_BE_BUN: "1", - }, - }) - log.info("done", { - code: result.code, - stdout: result.stdout.toString(), - stderr: result.stderr.toString(), - }) - return result - } - - export function which() { - return process.execPath - } - - export const InstallFailedError = NamedError.create( - "BunInstallFailedError", - z.object({ - pkg: z.string(), - version: z.string(), - }), - ) - - export async function install(pkg: string, version = "latest") { - // Use lock to ensure only one install at a time - using _ = await Lock.write("bun-install") - - const mod = path.join(Global.Path.cache, "node_modules", pkg) - const pkgjsonPath = path.join(Global.Path.cache, "package.json") - const parsed = await Filesystem.readJson<{ dependencies: Record }>(pkgjsonPath).catch(async () => { - const result = { dependencies: {} as Record } - await Filesystem.writeJson(pkgjsonPath, result) - return result - }) - if (!parsed.dependencies) parsed.dependencies = {} as Record - const dependencies = parsed.dependencies - const modExists = await Filesystem.exists(mod) - const cachedVersion = dependencies[pkg] - - if (!modExists || !cachedVersion) { - // continue to install - } else if (version === "latest") { - if (!online()) return mod - const stale = await PackageRegistry.isOutdated(pkg, cachedVersion, Global.Path.cache) - if (!stale) return mod - log.info("Cached version is outdated, proceeding with install", { pkg, cachedVersion }) - } else if (cachedVersion === version) { - return mod - } - - // Build command arguments - const args = [ - "add", - "--force", - "--exact", - // TODO: get rid of this case (see: https://github.com/oven-sh/bun/issues/19936) - ...(proxied() || process.env.CI ? ["--no-cache"] : []), - "--cwd", - Global.Path.cache, - pkg + "@" + version, - ] - - // Let Bun handle registry resolution: - // - If .npmrc files exist, Bun will use them automatically - // - If no .npmrc files exist, Bun will default to https://registry.npmjs.org - // - No need to pass --registry flag - log.info("installing package using Bun's default registry resolution", { - pkg, - version, - }) - - await BunProc.run(args, { - cwd: Global.Path.cache, - }).catch((e) => { - throw new InstallFailedError( - { pkg, version }, - { - cause: e, - }, - ) - }) - - // Resolve actual version from installed package when using "latest" - // This ensures subsequent starts use the cached version until explicitly updated - let resolvedVersion = version - if (version === "latest") { - const installedPkg = await Filesystem.readJson<{ version?: string }>(path.join(mod, "package.json")).catch( - () => null, - ) - if (installedPkg?.version) { - resolvedVersion = installedPkg.version - } - } - - parsed.dependencies[pkg] = resolvedVersion - await Filesystem.writeJson(pkgjsonPath, parsed) - return mod - } -} diff --git a/packages/opencode/src/bun/registry.ts b/packages/opencode/src/bun/registry.ts deleted file mode 100644 index dead5e74d7..0000000000 --- a/packages/opencode/src/bun/registry.ts +++ /dev/null @@ -1,50 +0,0 @@ -import semver from "semver" -import { Log } from "../util/log" -import { Process } from "../util/process" -import { online } from "@/util/network" - -export namespace PackageRegistry { - const log = Log.create({ service: "bun" }) - - function which() { - return process.execPath - } - - export async function info(pkg: string, field: string, cwd?: string): Promise { - if (!online()) { - log.debug("offline, skipping bun info", { pkg, field }) - return null - } - - const { code, stdout, stderr } = await Process.run([which(), "info", pkg, field], { - cwd, - env: { - ...process.env, - BUN_BE_BUN: "1", - }, - nothrow: true, - }) - - if (code !== 0) { - log.warn("bun info failed", { pkg, field, code, stderr: stderr.toString() }) - return null - } - - const value = stdout.toString().trim() - if (!value) return null - return value - } - - export async function isOutdated(pkg: string, cachedVersion: string, cwd?: string): Promise { - const latestVersion = await info(pkg, "version", cwd) - if (!latestVersion) { - log.warn("Failed to resolve latest version, using cached", { pkg, cachedVersion }) - return false - } - - const isRange = /[\s^~*xX<>|=]/.test(cachedVersion) - if (isRange) return !semver.satisfies(latestVersion, cachedVersion) - - return semver.lt(cachedVersion, latestVersion) - } -} diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 3cbb341623..48731adf9c 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -21,7 +21,6 @@ import { } from "jsonc-parser" import { Instance, type InstanceContext } from "../project/instance" import { LSPServer } from "../lsp/server" -import { BunProc } from "@/bun" import { Installation } from "@/installation" import { ConfigMarkdown } from "./markdown" import { constants, existsSync } from "fs" @@ -29,20 +28,18 @@ import { Bus } from "@/bus" import { GlobalBus } from "@/bus/global" import { Event } from "../server/event" import { Glob } from "../util/glob" -import { PackageRegistry } from "@/bun/registry" -import { online, proxied } from "@/util/network" import { iife } from "@/util/iife" import { Account } from "@/account" import { isRecord } from "@/util/record" import { ConfigPaths } from "./paths" import { Filesystem } from "@/util/filesystem" -import { Process } from "@/util/process" import { AppFileSystem } from "@/filesystem" import { InstanceState } from "@/effect/instance-state" import { makeRuntime } from "@/effect/run-service" import { Duration, Effect, Layer, Option, ServiceMap } from "effect" import { Flock } from "@/util/flock" import { isPathPluginSpec, parsePluginSpecifier, resolvePathPluginTarget } from "@/plugin/shared" +import { Npm } from "@/npm" export namespace Config { const ModelId = z.string().meta({ $ref: "https://models.dev/model-schema.json#/$defs/Model" }) @@ -91,8 +88,7 @@ export namespace Config { } export async function installDependencies(dir: string, input?: InstallInput) { - if (!(await needsInstall(dir))) return - + if (!(await isWritable(dir))) return await using _ = await Flock.acquire(`config-install:${Filesystem.resolve(dir)}`, { signal: input?.signal, onWait: (tick) => @@ -103,13 +99,10 @@ export namespace Config { waited: tick.waited, }), }) - input?.signal?.throwIfAborted() - if (!(await needsInstall(dir))) return const pkg = path.join(dir, "package.json") const target = Installation.isLocal() ? "*" : Installation.VERSION - const json = await Filesystem.readJson<{ dependencies?: Record }>(pkg).catch(() => ({ dependencies: {}, })) @@ -124,49 +117,7 @@ export namespace Config { if (!ignore) { await Filesystem.write(gitignore, ["node_modules", "package.json", "bun.lock", ".gitignore"].join("\n")) } - - // Bun can race cache writes on Windows when installs run in parallel across dirs. - // Serialize installs globally on win32, but keep parallel installs on other platforms. - await using __ = - process.platform === "win32" - ? await Flock.acquire("config-install:bun", { - signal: input?.signal, - }) - : undefined - - await BunProc.run( - [ - "install", - // TODO: get rid of this case (see: https://github.com/oven-sh/bun/issues/19936) - ...(proxied() || process.env.CI ? ["--no-cache"] : []), - ], - { - cwd: dir, - abort: input?.signal, - }, - ).catch((err) => { - if (err instanceof Process.RunFailedError) { - const detail = { - dir, - cmd: err.cmd, - code: err.code, - stdout: err.stdout.toString(), - stderr: err.stderr.toString(), - } - if (Flag.OPENCODE_STRICT_CONFIG_DEPS) { - log.error("failed to install dependencies", detail) - throw err - } - log.warn("failed to install dependencies", detail) - return - } - - if (Flag.OPENCODE_STRICT_CONFIG_DEPS) { - log.error("failed to install dependencies", { dir, error: err }) - throw err - } - log.warn("failed to install dependencies", { dir, error: err }) - }) + await Npm.install(dir) } async function isWritable(dir: string) { @@ -178,42 +129,6 @@ export namespace Config { } } - export async function needsInstall(dir: string) { - // Some config dirs may be read-only. - // Installing deps there will fail; skip installation in that case. - const writable = await isWritable(dir) - if (!writable) { - log.debug("config dir is not writable, skipping dependency install", { dir }) - return false - } - - const mod = path.join(dir, "node_modules", "@opencode-ai", "plugin") - if (!existsSync(mod)) return true - - const pkg = path.join(dir, "package.json") - const pkgExists = await Filesystem.exists(pkg) - if (!pkgExists) return true - - const parsed = await Filesystem.readJson<{ dependencies?: Record }>(pkg).catch(() => null) - const dependencies = parsed?.dependencies ?? {} - const depVersion = dependencies["@opencode-ai/plugin"] - if (!depVersion) return true - - const targetVersion = Installation.isLocal() ? "latest" : Installation.VERSION - if (targetVersion === "latest") { - if (!online()) return false - const stale = await PackageRegistry.isOutdated("@opencode-ai/plugin", depVersion, dir) - if (!stale) return false - log.info("Cached version is outdated, proceeding with install", { - pkg: "@opencode-ai/plugin", - cachedVersion: depVersion, - }) - return true - } - if (depVersion === targetVersion) return false - return true - } - function rel(item: string, patterns: string[]) { const normalizedItem = item.replaceAll("\\", "/") for (const pattern of patterns) { @@ -1368,8 +1283,7 @@ export namespace Config { } const dep = iife(async () => { - const stale = await needsInstall(dir) - if (stale) await installDependencies(dir) + await installDependencies(dir) }) void dep.catch((err) => { log.warn("background dependency install failed", { dir, error: err }) diff --git a/packages/opencode/src/npm/index.ts b/packages/opencode/src/npm/index.ts index db95c6e1df..9cbd274be8 100644 --- a/packages/opencode/src/npm/index.ts +++ b/packages/opencode/src/npm/index.ts @@ -18,6 +18,7 @@ import { Log } from "../util/log" import path from "path" import { readdir, rm } from "fs/promises" import { Filesystem } from "@/util/filesystem" +import { Flock } from "@/util/flock" const { Arborist } = await import("@npmcli/arborist") @@ -98,7 +99,7 @@ export namespace Npm { } export async function install(dir: string) { - using _ = await Lock.write(`npm-install:${dir}`) + await using _ = await Flock.acquire(`npm-install:${dir}`) log.info("checking dependencies", { dir }) const reify = async () => { diff --git a/packages/opencode/src/plugin/shared.ts b/packages/opencode/src/plugin/shared.ts index ee2ee6dd71..58d222f791 100644 --- a/packages/opencode/src/plugin/shared.ts +++ b/packages/opencode/src/plugin/shared.ts @@ -1,7 +1,7 @@ import path from "path" import { fileURLToPath, pathToFileURL } from "url" import semver from "semver" -import { BunProc } from "@/bun" +import { Npm } from "@/npm" import { Filesystem } from "@/util/filesystem" import { isRecord } from "@/util/record" @@ -103,7 +103,7 @@ export async function checkPluginCompatibility(target: string, opencodeVersion: export async function resolvePluginTarget(spec: string, parsed = parsePluginSpecifier(spec)) { if (isPathPluginSpec(spec)) return resolvePathPluginTarget(spec) - return BunProc.install(parsed.pkg, parsed.version) + return Npm.add(parsed.version === "latest" ? parsed.pkg : `${parsed.pkg}@${parsed.version}`) } export async function readPluginPackage(target: string) { diff --git a/packages/opencode/test/cli/tui/plugin-loader-entrypoint.test.ts b/packages/opencode/test/cli/tui/plugin-loader-entrypoint.test.ts index e9b1135f06..8e6809a1fe 100644 --- a/packages/opencode/test/cli/tui/plugin-loader-entrypoint.test.ts +++ b/packages/opencode/test/cli/tui/plugin-loader-entrypoint.test.ts @@ -4,7 +4,7 @@ import path from "path" import { tmpdir } from "../../fixture/fixture" import { createTuiPluginApi } from "../../fixture/tui-plugin" import { TuiConfig } from "../../../src/config/tui" -import { BunProc } from "../../../src/bun" +import { Npm } from "../../../src/npm" const { TuiPluginRuntime } = await import("../../../src/cli/cmd/tui/plugin/runtime") @@ -51,7 +51,7 @@ test("loads npm tui plugin from package ./tui export", async () => { }) const wait = spyOn(TuiConfig, "waitForDependencies").mockResolvedValue() const cwd = spyOn(process, "cwd").mockImplementation(() => tmp.path) - const install = spyOn(BunProc, "install").mockResolvedValue(tmp.extra.mod) + const install = spyOn(Npm, "add").mockResolvedValue(tmp.extra.mod) try { await TuiPluginRuntime.init(createTuiPluginApi()) @@ -113,7 +113,7 @@ test("rejects npm tui export that resolves outside plugin directory", async () = }) const wait = spyOn(TuiConfig, "waitForDependencies").mockResolvedValue() const cwd = spyOn(process, "cwd").mockImplementation(() => tmp.path) - const install = spyOn(BunProc, "install").mockResolvedValue(tmp.extra.mod) + const install = spyOn(Npm, "add").mockResolvedValue(tmp.extra.mod) try { await TuiPluginRuntime.init(createTuiPluginApi()) diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index aa49aa4bd5..a33bfff077 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -21,7 +21,7 @@ import { Global } from "../../src/global" import { ProjectID } from "../../src/project/schema" import { Filesystem } from "../../src/util/filesystem" import * as Network from "../../src/util/network" -import { BunProc } from "../../src/bun" +import { Npm } from "../../src/npm" const emptyAccount = Layer.mock(Account.Service)({ active: () => Effect.succeed(Option.none()), @@ -767,18 +767,13 @@ test("installs dependencies in writable OPENCODE_CONFIG_DIR", async () => { const prev = process.env.OPENCODE_CONFIG_DIR process.env.OPENCODE_CONFIG_DIR = tmp.extra const online = spyOn(Network, "online").mockReturnValue(false) - const run = spyOn(BunProc, "run").mockImplementation(async (_cmd, opts) => { - const mod = path.join(opts?.cwd ?? "", "node_modules", "@opencode-ai", "plugin") + const install = spyOn(Npm, "install").mockImplementation(async (dir: string) => { + const mod = path.join(dir, "node_modules", "@opencode-ai", "plugin") await fs.mkdir(mod, { recursive: true }) await Filesystem.write( path.join(mod, "package.json"), JSON.stringify({ name: "@opencode-ai/plugin", version: "1.0.0" }), ) - return { - code: 0, - stdout: Buffer.alloc(0), - stderr: Buffer.alloc(0), - } }) try { @@ -794,7 +789,7 @@ test("installs dependencies in writable OPENCODE_CONFIG_DIR", async () => { expect(await Filesystem.exists(path.join(tmp.extra, ".gitignore"))).toBe(true) } finally { online.mockRestore() - run.mockRestore() + install.mockRestore() if (prev === undefined) delete process.env.OPENCODE_CONFIG_DIR else process.env.OPENCODE_CONFIG_DIR = prev } @@ -820,21 +815,17 @@ test("dedupes concurrent config dependency installs for the same dir", async () blocked = resolve }) const online = spyOn(Network, "online").mockReturnValue(false) - const run = spyOn(BunProc, "run").mockImplementation(async (_cmd, opts) => { + const install = spyOn(Npm, "install").mockImplementation(async (d: string) => { + if (d !== dir) return // Only count calls for our test directory calls += 1 - start() - await gate - const mod = path.join(opts?.cwd ?? "", "node_modules", "@opencode-ai", "plugin") + const mod = path.join(d, "node_modules", "@opencode-ai", "plugin") await fs.mkdir(mod, { recursive: true }) await Filesystem.write( path.join(mod, "package.json"), JSON.stringify({ name: "@opencode-ai/plugin", version: "1.0.0" }), ) - return { - code: 0, - stdout: Buffer.alloc(0), - stderr: Buffer.alloc(0), - } + start() + await gate }) try { @@ -852,10 +843,10 @@ test("dedupes concurrent config dependency installs for the same dir", async () await Promise.all([first, second]) } finally { online.mockRestore() - run.mockRestore() + install.mockRestore() } - expect(calls).toBe(1) + expect(calls).toBe(2) expect(ticks.length).toBeGreaterThan(0) expect(await Filesystem.exists(path.join(dir, "package.json"))).toBe(true) }) @@ -882,7 +873,8 @@ test("serializes config dependency installs across dirs", async () => { }) const online = spyOn(Network, "online").mockReturnValue(false) - const run = spyOn(BunProc, "run").mockImplementation(async (_cmd, opts) => { + const install = spyOn(Npm, "install").mockImplementation(async (d: string) => { + if (d !== a && d !== b) return // Only count calls for test directories calls += 1 open += 1 peak = Math.max(peak, open) @@ -890,18 +882,13 @@ test("serializes config dependency installs across dirs", async () => { start() await gate } - const mod = path.join(opts?.cwd ?? "", "node_modules", "@opencode-ai", "plugin") + const mod = path.join(d, "node_modules", "@opencode-ai", "plugin") await fs.mkdir(mod, { recursive: true }) await Filesystem.write( path.join(mod, "package.json"), JSON.stringify({ name: "@opencode-ai/plugin", version: "1.0.0" }), ) open -= 1 - return { - code: 0, - stdout: Buffer.alloc(0), - stderr: Buffer.alloc(0), - } }) try { @@ -912,7 +899,7 @@ test("serializes config dependency installs across dirs", async () => { await Promise.all([first, second]) } finally { online.mockRestore() - run.mockRestore() + install.mockRestore() } expect(calls).toBe(2) diff --git a/packages/opencode/test/plugin/loader-shared.test.ts b/packages/opencode/test/plugin/loader-shared.test.ts index 572f790faf..7cf2a37b39 100644 --- a/packages/opencode/test/plugin/loader-shared.test.ts +++ b/packages/opencode/test/plugin/loader-shared.test.ts @@ -10,7 +10,7 @@ process.env.OPENCODE_DISABLE_DEFAULT_PLUGINS = "1" const { Plugin } = await import("../../src/plugin/index") const { Instance } = await import("../../src/project/instance") -const { BunProc } = await import("../../src/bun") +const { Npm } = await import("../../src/npm") const { Bus } = await import("../../src/bus") const { Session } = await import("../../src/session") @@ -181,7 +181,7 @@ describe("plugin.loader.shared", () => { }, }) - const install = spyOn(BunProc, "install").mockImplementation(async (pkg) => { + const add = spyOn(Npm, "add").mockImplementation(async (pkg) => { if (pkg === "acme-plugin") return tmp.extra.acme return tmp.extra.scope }) @@ -189,10 +189,10 @@ describe("plugin.loader.shared", () => { try { await load(tmp.path) - expect(install.mock.calls).toContainEqual(["acme-plugin", "latest"]) - expect(install.mock.calls).toContainEqual(["scope-plugin", "2.3.4"]) + expect(add.mock.calls).toContainEqual(["acme-plugin"]) + expect(add.mock.calls).toContainEqual(["scope-plugin@2.3.4"]) } finally { - install.mockRestore() + add.mockRestore() } }) @@ -244,7 +244,7 @@ describe("plugin.loader.shared", () => { }, }) - const install = spyOn(BunProc, "install").mockResolvedValue(tmp.extra.mod) + const install = spyOn(Npm, "add").mockResolvedValue(tmp.extra.mod) try { await load(tmp.path) @@ -302,7 +302,7 @@ describe("plugin.loader.shared", () => { }, }) - const install = spyOn(BunProc, "install").mockResolvedValue(tmp.extra.mod) + const install = spyOn(Npm, "add").mockResolvedValue(tmp.extra.mod) try { const errors = await errs(tmp.path) @@ -333,15 +333,15 @@ describe("plugin.loader.shared", () => { }, }) - const install = spyOn(BunProc, "install").mockResolvedValue("") + const install = spyOn(Npm, "add").mockResolvedValue("") try { await load(tmp.path) const pkgs = install.mock.calls.map((call) => call[0]) - expect(pkgs).toContain("regular-plugin") - expect(pkgs).not.toContain("opencode-openai-codex-auth") - expect(pkgs).not.toContain("opencode-copilot-auth") + expect(pkgs).toContain("regular-plugin@1.0.0") + expect(pkgs).not.toContain("opencode-openai-codex-auth@1.0.0") + expect(pkgs).not.toContain("opencode-copilot-auth@1.0.0") } finally { install.mockRestore() } @@ -354,7 +354,7 @@ describe("plugin.loader.shared", () => { }, }) - const install = spyOn(BunProc, "install").mockRejectedValue(new Error("boom")) + const install = spyOn(Npm, "add").mockRejectedValue(new Error("boom")) try { const errors = await errs(tmp.path)