From b5c3bd7eff656f694a83cc776d092ffba3596a62 Mon Sep 17 00:00:00 2001 From: Dax Raad Date: Wed, 1 Apr 2026 11:59:38 -0400 Subject: [PATCH] fix: replace BunProc with Npm module - remove bun/index.ts and bun.test.ts - update resolvePluginTarget to use Npm.add instead of Npm.install - replace BunProc.install with Npm.add in provider.ts - update all tests to use Npm.add/Npm.install instead of BunProc --- packages/opencode/src/bun/index.ts | 129 ----------------- packages/opencode/src/plugin/shared.ts | 8 +- packages/opencode/src/provider/provider.ts | 2 +- packages/opencode/test/bun.test.ts | 137 ------------------ .../cli/tui/plugin-loader-entrypoint.test.ts | 8 +- packages/opencode/test/config/config.test.ts | 28 ++-- .../test/plugin/loader-shared.test.ts | 10 +- 7 files changed, 27 insertions(+), 295 deletions(-) delete mode 100644 packages/opencode/src/bun/index.ts delete mode 100644 packages/opencode/test/bun.test.ts diff --git a/packages/opencode/src/bun/index.ts b/packages/opencode/src/bun/index.ts deleted file mode 100644 index 589414a025..0000000000 --- a/packages/opencode/src/bun/index.ts +++ /dev/null @@ -1,129 +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", opts?: { ignoreScripts?: boolean }) { - // 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", - ...(opts?.ignoreScripts ? ["--ignore-scripts"] : []), - // 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/plugin/shared.ts b/packages/opencode/src/plugin/shared.ts index 110ef878bc..95b18085bd 100644 --- a/packages/opencode/src/plugin/shared.ts +++ b/packages/opencode/src/plugin/shared.ts @@ -106,7 +106,7 @@ async function resolveDirectoryIndex(dir: string) { async function resolveTargetDirectory(target: string) { const file = targetPath(target) if (!file) return - const stat = await Filesystem.stat(file) + const stat = Filesystem.stat(file) if (!stat?.isDirectory()) return return file } @@ -153,7 +153,7 @@ export function isPathPluginSpec(spec: string) { export async function resolvePathPluginTarget(spec: string) { const raw = spec.startsWith("file://") ? fileURLToPath(spec) : spec const file = path.isAbsolute(raw) || /^[A-Za-z]:[\\/]/.test(raw) ? raw : path.resolve(raw) - const stat = await Filesystem.stat(file) + const stat = Filesystem.stat(file) if (!stat?.isDirectory()) { if (spec.startsWith("file://")) return spec return pathToFileURL(file).href @@ -184,12 +184,12 @@ 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, { ignoreScripts: true }) + return Npm.add(parsed.pkg + "@" + parsed.version) } export async function readPluginPackage(target: string): Promise { const file = target.startsWith("file://") ? fileURLToPath(target) : target - const stat = await Filesystem.stat(file) + const stat = Filesystem.stat(file) const dir = stat?.isDirectory() ? file : path.dirname(file) const pkg = path.join(dir, "package.json") const json = await Filesystem.readJson>(pkg) diff --git a/packages/opencode/src/provider/provider.ts b/packages/opencode/src/provider/provider.ts index 22d2a0c507..4260158693 100644 --- a/packages/opencode/src/provider/provider.ts +++ b/packages/opencode/src/provider/provider.ts @@ -1364,7 +1364,7 @@ export namespace Provider { let installedPath: string if (!model.api.npm.startsWith("file://")) { - installedPath = await BunProc.install(model.api.npm, "latest") + installedPath = await Npm.add(model.api.npm) } else { log.info("loading local provider", { pkg: model.api.npm }) installedPath = model.api.npm diff --git a/packages/opencode/test/bun.test.ts b/packages/opencode/test/bun.test.ts deleted file mode 100644 index db3fa2a28c..0000000000 --- a/packages/opencode/test/bun.test.ts +++ /dev/null @@ -1,137 +0,0 @@ -import { describe, expect, spyOn, test } from "bun:test" -import fs from "fs/promises" -import path from "path" -import { BunProc } from "../src/bun" -import { PackageRegistry } from "../src/bun/registry" -import { Global } from "../src/global" -import { Process } from "../src/util/process" - -describe("BunProc registry configuration", () => { - test("should not contain hardcoded registry parameters", async () => { - // Read the bun/index.ts file - const bunIndexPath = path.join(__dirname, "../src/bun/index.ts") - const content = await fs.readFile(bunIndexPath, "utf-8") - - // Verify that no hardcoded registry is present - expect(content).not.toContain("--registry=") - expect(content).not.toContain("hasNpmRcConfig") - expect(content).not.toContain("NpmRc") - }) - - test("should use Bun's default registry resolution", async () => { - // Read the bun/index.ts file - const bunIndexPath = path.join(__dirname, "../src/bun/index.ts") - const content = await fs.readFile(bunIndexPath, "utf-8") - - // Verify that it uses Bun's default resolution - expect(content).toContain("Bun's default registry resolution") - expect(content).toContain("Bun will use them automatically") - expect(content).toContain("No need to pass --registry flag") - }) - - test("should have correct command structure without registry", async () => { - // Read the bun/index.ts file - const bunIndexPath = path.join(__dirname, "../src/bun/index.ts") - const content = await fs.readFile(bunIndexPath, "utf-8") - - // Extract the install function - const installFunctionMatch = content.match(/export async function install[\s\S]*?^ }/m) - expect(installFunctionMatch).toBeTruthy() - - if (installFunctionMatch) { - const installFunction = installFunctionMatch[0] - - // Verify expected arguments are present - expect(installFunction).toContain('"add"') - expect(installFunction).toContain('"--force"') - expect(installFunction).toContain('"--exact"') - expect(installFunction).toContain('"--cwd"') - expect(installFunction).toContain("Global.Path.cache") - expect(installFunction).toContain('pkg + "@" + version') - - // Verify no registry argument is added - expect(installFunction).not.toContain('"--registry"') - expect(installFunction).not.toContain('args.push("--registry') - } - }) -}) - -describe("BunProc install pinning", () => { - test("uses pinned cache without touching registry", async () => { - const pkg = `pin-test-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}` - const ver = "1.2.3" - const mod = path.join(Global.Path.cache, "node_modules", pkg) - const data = path.join(Global.Path.cache, "package.json") - - await fs.mkdir(mod, { recursive: true }) - await Bun.write(path.join(mod, "package.json"), JSON.stringify({ name: pkg, version: ver }, null, 2)) - - const src = await fs.readFile(data, "utf8").catch(() => "") - const json = src ? ((JSON.parse(src) as { dependencies?: Record }) ?? {}) : {} - const deps = json.dependencies ?? {} - deps[pkg] = ver - await Bun.write(data, JSON.stringify({ ...json, dependencies: deps }, null, 2)) - - const stale = spyOn(PackageRegistry, "isOutdated").mockImplementation(async () => { - throw new Error("unexpected registry check") - }) - const run = spyOn(Process, "run").mockImplementation(async () => { - throw new Error("unexpected process.run") - }) - - try { - const out = await BunProc.install(pkg, ver) - expect(out).toBe(mod) - expect(stale).not.toHaveBeenCalled() - expect(run).not.toHaveBeenCalled() - } finally { - stale.mockRestore() - run.mockRestore() - - await fs.rm(mod, { recursive: true, force: true }) - const end = await fs - .readFile(data, "utf8") - .then((item) => JSON.parse(item) as { dependencies?: Record }) - .catch(() => undefined) - if (end?.dependencies) { - delete end.dependencies[pkg] - await Bun.write(data, JSON.stringify(end, null, 2)) - } - } - }) - - test("passes --ignore-scripts when requested", async () => { - const pkg = `ignore-test-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}` - const ver = "4.5.6" - const mod = path.join(Global.Path.cache, "node_modules", pkg) - const data = path.join(Global.Path.cache, "package.json") - - const run = spyOn(Process, "run").mockImplementation(async () => ({ - code: 0, - stdout: Buffer.alloc(0), - stderr: Buffer.alloc(0), - })) - - try { - await fs.rm(mod, { recursive: true, force: true }) - await BunProc.install(pkg, ver, { ignoreScripts: true }) - - expect(run).toHaveBeenCalled() - const call = run.mock.calls[0]?.[0] - expect(call).toContain("--ignore-scripts") - expect(call).toContain(`${pkg}@${ver}`) - } finally { - run.mockRestore() - await fs.rm(mod, { recursive: true, force: true }) - - const end = await fs - .readFile(data, "utf8") - .then((item) => JSON.parse(item) as { dependencies?: Record }) - .catch(() => undefined) - if (end?.dependencies) { - delete end.dependencies[pkg] - await Bun.write(data, JSON.stringify(end, null, 2)) - } - } - }) -}) 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 51f90c48ea..f376e539f5 100644 --- a/packages/opencode/test/cli/tui/plugin-loader-entrypoint.test.ts +++ b/packages/opencode/test/cli/tui/plugin-loader-entrypoint.test.ts @@ -118,7 +118,7 @@ test("does not use npm package exports dot for tui entry", 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()) @@ -244,7 +244,7 @@ test("rejects npm tui plugin that exports server and tui together", 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()) @@ -303,7 +303,7 @@ test("does not use npm package main for tui entry", 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) const warn = spyOn(console, "warn").mockImplementation(() => {}) const error = spyOn(console, "error").mockImplementation(() => {}) @@ -475,7 +475,7 @@ test("uses npm package name when tui plugin id is omitted", 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 941a6edf40..6369ab5cee 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -816,21 +816,24 @@ 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 hit = path.normalize(opts?.cwd ?? "") === path.normalize(dir) + const targetDir = dir + const run = spyOn(Npm, "install").mockImplementation(async (d: string) => { + const hit = path.normalize(d) === path.normalize(targetDir) if (hit) { 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" }), ) - start() - await gate + if (hit) { + start() + await gate + } }) try { @@ -848,7 +851,7 @@ test("dedupes concurrent config dependency installs for the same dir", async () await Promise.all([first, second]) } finally { online.mockRestore() - install.mockRestore() + run.mockRestore() } expect(calls).toBe(2) @@ -878,8 +881,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 cwd = path.normalize(opts?.cwd ?? "") + const run = spyOn(Npm, "install").mockImplementation(async (dir: string) => { + const cwd = path.normalize(dir) const hit = cwd === path.normalize(a) || cwd === path.normalize(b) if (hit) { calls += 1 @@ -890,7 +893,7 @@ test("serializes config dependency installs across dirs", async () => { await gate } } - const mod = path.join(d, "node_modules", "@opencode-ai", "plugin") + const mod = path.join(cwd, "node_modules", "@opencode-ai", "plugin") await fs.mkdir(mod, { recursive: true }) await Filesystem.write( path.join(mod, "package.json"), @@ -899,11 +902,6 @@ test("serializes config dependency installs across dirs", async () => { if (hit) { open -= 1 } - return { - code: 0, - stdout: Buffer.alloc(0), - stderr: Buffer.alloc(0), - } }) try { @@ -914,7 +912,7 @@ test("serializes config dependency installs across dirs", async () => { await Promise.all([first, second]) } finally { online.mockRestore() - install.mockRestore() + run.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 5f29cb45a0..1467d186e7 100644 --- a/packages/opencode/test/plugin/loader-shared.test.ts +++ b/packages/opencode/test/plugin/loader-shared.test.ts @@ -266,8 +266,8 @@ describe("plugin.loader.shared", () => { try { await load(tmp.path) - expect(install.mock.calls).toContainEqual(["acme-plugin", "latest", { ignoreScripts: true }]) - expect(install.mock.calls).toContainEqual(["scope-plugin", "2.3.4", { ignoreScripts: true }]) + expect(add.mock.calls).toContainEqual(["acme-plugin@latest"]) + expect(add.mock.calls).toContainEqual(["scope-plugin@2.3.4"]) } finally { add.mockRestore() } @@ -378,7 +378,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) @@ -431,7 +431,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) @@ -477,7 +477,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)