From a617f790f106b69b6aff22fe03f3a38286883974 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Fri, 17 Apr 2026 13:41:33 +0800 Subject: [PATCH] fix(opencode): harden config-scoped dependency loading --- packages/opencode/src/config/config.ts | 9 ++- packages/opencode/src/config/dependency.ts | 60 +++++++++++++++++++ packages/opencode/src/plugin/index.ts | 42 ++++--------- packages/opencode/src/tool/registry.ts | 31 +--------- packages/opencode/test/config/config.test.ts | 33 ++++++++++ .../test/plugin/loader-shared.test.ts | 43 +++++++++++++ packages/opencode/test/tool/registry.test.ts | 44 ++++++++++++++ 7 files changed, 199 insertions(+), 63 deletions(-) create mode 100644 packages/opencode/src/config/dependency.ts diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 28370931f..62b22d8d6 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -173,14 +173,17 @@ export namespace Config { ) const dependencies: Record = json.dependencies ?? {} const hasDep = dependencies["@opencode-ai/plugin"] === target - json.dependencies = { + const required = { ...dependencies, "@opencode-ai/plugin": target, } + json.dependencies = required const gitignore = path.join(dir, ".gitignore") const ignore = await Filesystem.exists(gitignore) - const hasPkg = await Filesystem.exists(plugin) + const installed = await Promise.all( + Object.keys(required).map((pkg) => Filesystem.exists(path.join(dir, "node_modules", ...pkg.split("/"), "package.json"))), + ) if (!hasDep) { await Filesystem.writeJson(pkg, json) } @@ -190,7 +193,7 @@ export namespace Config { ["node_modules", "package.json", "package-lock.json", "bun.lock", ".gitignore"].join("\n"), ) } - if (hasDep && ignore && hasPkg) return + if (hasDep && ignore && installed.every(Boolean)) return await Npm.install(dir) } diff --git a/packages/opencode/src/config/dependency.ts b/packages/opencode/src/config/dependency.ts new file mode 100644 index 000000000..eb8e14ed5 --- /dev/null +++ b/packages/opencode/src/config/dependency.ts @@ -0,0 +1,60 @@ +import path from "path" +import { builtinModules, isBuiltin } from "module" +import { Filesystem } from "@/util/filesystem" + +const DEPENDENCY_IMPORT = + /(?:^|\n)\s*(?:import\s+(?:[^"'`]+\s+from\s+)?|export\s+[^"'`]+\s+from\s+)["']([^./"'`][^"'`]*)["']|import\s*\(\s*["']([^./"'`][^"'`]*)["']\s*\)|require\(\s*["']([^./"'`][^"'`]*)["']\s*\)/gm +const LOCAL_IMPORT = + /(?:^|\n)\s*(?:import\s+(?:[^"'`]+\s+from\s+)?|export\s+[^"'`]+\s+from\s+)["']((?:\.\.?\/)[^"'`]*)["']|import\s*\(\s*["']((?:\.\.?\/)[^"'`]*)["']\s*\)|require\(\s*["']((?:\.\.?\/)[^"'`]*)["']\s*\)/gm +const BUILTIN_MODULES = new Set(builtinModules) +const LOCAL_IMPORT_EXTENSIONS = [".ts", ".tsx", ".js", ".jsx", ".mjs", ".cjs"] + +function packageName(spec: string) { + if (spec.startsWith("node:") || isBuiltin(spec) || BUILTIN_MODULES.has(spec)) return + if (spec.startsWith("@")) { + const [scope, name] = spec.split("/") + if (!scope || !name) return + return `${scope}/${name}` + } + return spec.split("/")[0] +} + +async function resolveLocalImport(file: string, spec: string) { + const target = path.resolve(path.dirname(file), spec) + const candidates = path.extname(target) + ? [target] + : [ + ...LOCAL_IMPORT_EXTENSIONS.map((ext) => `${target}${ext}`), + ...LOCAL_IMPORT_EXTENSIONS.map((ext) => path.join(target, `index${ext}`)), + ] + for (const candidate of candidates) { + if (await Filesystem.exists(candidate)) return candidate + } +} + +export async function needsConfigDependencies(file: string, configDir: string, visited = new Set()) { + const resolved = path.resolve(file) + if (visited.has(resolved)) return false + visited.add(resolved) + + const text = await Filesystem.readText(resolved).catch(() => "") + for (const match of text.matchAll(DEPENDENCY_IMPORT)) { + const spec = match[1] ?? match[2] ?? match[3] + if (!spec) continue + const pkg = packageName(spec) + if (!pkg) continue + const pkgPath = path.join(configDir, "node_modules", ...pkg.split("/")) + if (await Filesystem.exists(path.join(pkgPath, "package.json"))) continue + return true + } + + for (const match of text.matchAll(LOCAL_IMPORT)) { + const spec = match[1] ?? match[2] ?? match[3] + if (!spec) continue + const next = await resolveLocalImport(resolved, spec) + if (!next) continue + if (await needsConfigDependencies(next, configDir, visited)) return true + } + + return false +} diff --git a/packages/opencode/src/plugin/index.ts b/packages/opencode/src/plugin/index.ts index cc3588e72..0e7fb5a37 100644 --- a/packages/opencode/src/plugin/index.ts +++ b/packages/opencode/src/plugin/index.ts @@ -18,26 +18,12 @@ import { EffectLogger } from "@/effect/logger" import { InstanceState } from "@/effect/instance-state" import { makeRuntime } from "@/effect/run-service" import { errorMessage } from "@/util/error" -import { Filesystem } from "@/util/filesystem" +import { needsConfigDependencies } from "@/config/dependency" import { PluginLoader } from "./loader" import { parsePluginSpecifier, readPluginId, readV1Plugin, resolvePluginId } from "./shared" -import { builtinModules, isBuiltin } from "module" export namespace Plugin { const log = Log.create({ service: "plugin" }) - const DEPENDENCY_IMPORT = - /(?:^|\n)\s*(?:import\s+(?:[^"'`]+\s+from\s+)?|export\s+[^"'`]+\s+from\s+)["']([^./"'`][^"'`]*)["']|import\s*\(\s*["']([^./"'`][^"'`]*)["']\s*\)|require\(\s*["']([^./"'`][^"'`]*)["']\s*\)/gm - const BUILTIN_MODULES = new Set(builtinModules) - - function packageName(spec: string) { - if (spec.startsWith("node:") || isBuiltin(spec) || BUILTIN_MODULES.has(spec)) return - if (spec.startsWith("@")) { - const [scope, name] = spec.split("/") - if (!scope || !name) return - return `${scope}/${name}` - } - return spec.split("/")[0] - } function dependencyDir(source: string, projectDir: string) { if (source === "OPENCODE_CONFIG_CONTENT") return projectDir @@ -45,21 +31,6 @@ export namespace Plugin { return source } - async function needsConfigDependencies(file: string, source: string, projectDir: string) { - const text = await Filesystem.readText(file).catch(() => "") - const configDir = dependencyDir(source, projectDir) - for (const match of text.matchAll(DEPENDENCY_IMPORT)) { - const spec = match[1] ?? match[2] ?? match[3] - if (!spec) continue - const pkg = packageName(spec) - if (!pkg) continue - const pkgPath = path.join(configDir, "node_modules", ...pkg.split("/")) - if (await Filesystem.exists(path.join(pkgPath, "package.json"))) continue - return true - } - return false - } - type State = { hooks: Hooks[] } @@ -192,6 +163,15 @@ export namespace Plugin { if (Flag.OPENCODE_PURE && cfg.plugin_origins?.length) { log.info("skipping external plugins in pure mode", { count: cfg.plugin_origins.length }) } + for (const origin of plugins) { + const spec = Config.pluginSpecifier(origin.spec) + if (!spec.startsWith("file://")) continue + if (!(yield* Effect.promise(() => needsConfigDependencies(fileURLToPath(spec), dependencyDir(origin.source, ctx.directory))))) { + continue + } + yield* Effect.promise(() => Config.waitForDependencies().catch(() => undefined)) + break + } const loaded = yield* Effect.promise(() => PluginLoader.loadExternal({ @@ -201,7 +181,7 @@ export namespace Plugin { shouldRetry: (origin) => { const spec = Config.pluginSpecifier(origin.spec) if (!spec.startsWith("file://")) return Promise.resolve(false) - return needsConfigDependencies(fileURLToPath(spec), origin.source, ctx.directory) + return needsConfigDependencies(fileURLToPath(spec), dependencyDir(origin.source, ctx.directory)) }, report: { start(candidate) { diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index 37c8d50c6..4d521f131 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -49,37 +49,10 @@ import { AppFileSystem } from "../filesystem" import { Bus } from "../bus" import { Agent } from "../agent/agent" import { Skill } from "../skill" -import { builtinModules, isBuiltin } from "module" -import { Filesystem } from "../util/filesystem" +import { needsConfigDependencies } from "../config/dependency" export namespace ToolRegistry { const log = Log.create({ service: "tool.registry" }) - const DEPENDENCY_IMPORT = - /(?:^|\n)\s*(?:import\s+(?:[^"'`]+\s+from\s+)?|export\s+[^"'`]+\s+from\s+)["']([^./"'`][^"'`]*)["']|import\s*\(\s*["']([^./"'`][^"'`]*)["']\s*\)|require\(\s*["']([^./"'`][^"'`]*)["']\s*\)/gm - const BUILTIN_MODULES = new Set(builtinModules) - - function packageName(spec: string) { - if (spec.startsWith("node:") || isBuiltin(spec) || BUILTIN_MODULES.has(spec)) return - if (spec.startsWith("@")) { - const [scope, name] = spec.split("/") - if (!scope || !name) return - return `${scope}/${name}` - } - return spec.split("/")[0] - } - - async function needsConfigDependencies(text: string, dir: string) { - for (const match of text.matchAll(DEPENDENCY_IMPORT)) { - const spec = match[1] ?? match[2] ?? match[3] - if (!spec) continue - const pkg = packageName(spec) - if (!pkg) continue - const pkgPath = path.join(dir, "node_modules", ...pkg.split("/")) - if (await Filesystem.exists(path.join(pkgPath, "package.json"))) continue - return true - } - return false - } type TaskDef = Tool.InferDef type ReadDef = Tool.InferDef @@ -206,7 +179,7 @@ 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)))))) { + if (!depsReady && (yield* Effect.promise(() => needsConfigDependencies(match, path.dirname(path.dirname(match)))))) { depsReady = true yield* config.waitForDependencies() } diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index 398d811d8..963c36adc 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -953,6 +953,39 @@ test("skips reinstall when config dependencies are already bootstrapped", async } }) +test("reinstalls when declared config dependencies are missing from node_modules", async () => { + await using tmp = await tmpdir() + const dir = path.join(tmp.path, "configdir") + await fs.mkdir(path.join(dir, "node_modules", "@opencode-ai", "plugin"), { recursive: true }) + const target = Installation.isLocal() ? "*" : Installation.VERSION + await Filesystem.writeJson(path.join(dir, "package.json"), { + dependencies: { + "@opencode-ai/plugin": target, + "late-dep": "^1.0.0", + }, + }) + await Filesystem.write( + path.join(dir, ".gitignore"), + ["node_modules", "package.json", "package-lock.json", "bun.lock", ".gitignore"].join("\n"), + ) + await Filesystem.writeJson(path.join(dir, "node_modules", "@opencode-ai", "plugin", "package.json"), { + name: "@opencode-ai/plugin", + version: "1.0.0", + type: "module", + exports: "./index.js", + }) + + const install = spyOn(Npm, "install").mockImplementation(async (cwd: string) => writeMockConfigInstall(cwd)) + + try { + await expect(Config.installDependencies(dir)).resolves.toBeUndefined() + expect(install).toHaveBeenCalledTimes(1) + await expect(Filesystem.exists(path.join(dir, "node_modules", "late-dep", "package.json"))).resolves.toBe(true) + } finally { + install.mockRestore() + } +}) + test("resolves scoped npm plugins in config", async () => { await using tmp = await tmpdir({ init: async (dir) => { diff --git a/packages/opencode/test/plugin/loader-shared.test.ts b/packages/opencode/test/plugin/loader-shared.test.ts index 0187b832e..1637d9267 100644 --- a/packages/opencode/test/plugin/loader-shared.test.ts +++ b/packages/opencode/test/plugin/loader-shared.test.ts @@ -14,6 +14,7 @@ const { readPackageThemes } = await import("../../src/plugin/shared") const { Instance } = await import("../../src/project/instance") const { Npm } = await import("../../src/npm") const { Config } = await import("../../src/config/config") +const { writeMockConfigInstall } = await import("../shared/mock-npm-install") afterAll(() => { if (disableDefault === undefined) { @@ -748,6 +749,48 @@ describe("plugin.loader.shared", () => { } }) + test("retries auto-discovered file plugins that reach config deps through helper imports", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + const pluginsDir = path.join(dir, ".opencode", "plugins") + const pluginFile = path.join(pluginsDir, "plugin.ts") + const helperFile = path.join(pluginsDir, "helper.ts") + const mark = path.join(dir, "plugin.txt") + + await fs.mkdir(pluginsDir, { recursive: true }) + await Bun.write( + helperFile, + ["import { ready } from 'late-dep'", "export { ready }", ""].join("\n"), + ) + await Bun.write( + pluginFile, + [ + "import { ready } from './helper'", + "export default {", + ' id: "demo.helper",', + " server: async () => {", + ` await Bun.write(${JSON.stringify(mark)}, ready)`, + " return {}", + " },", + "}", + "", + ].join("\n"), + ) + + return { mark } + }, + }) + + const install = spyOn(Npm, "install").mockImplementation(async (dir: string) => writeMockConfigInstall(dir)) + + try { + await load(tmp.path) + expect(await Bun.file(tmp.extra.mark).text()).toBe("hello") + } finally { + install.mockRestore() + } + }) + test("loads object plugin via plugin.server", async () => { await using tmp = await tmpdir({ init: async (dir) => { diff --git a/packages/opencode/test/tool/registry.test.ts b/packages/opencode/test/tool/registry.test.ts index 0fd341380..9363da1f2 100644 --- a/packages/opencode/test/tool/registry.test.ts +++ b/packages/opencode/test/tool/registry.test.ts @@ -196,6 +196,50 @@ describe("tool.registry", () => { } }) + test("waits for config-scoped dependencies used through local helper 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, "helper.ts"), + ["import { ready } from 'late-dep'", "export { ready }", ""].join("\n"), + ) + + await Bun.write( + path.join(toolsDir, "late.ts"), + [ + "import { ready } from './helper'", + "export default {", + " description: 'tool that waits for helper dependencies',", + " args: {},", + " execute: async () => ready,", + "}", + "", + ].join("\n"), + ) + }, + }) + + const install = spyOn(Npm, "install").mockImplementation((dir: string) => writeMockConfigInstall(dir)) + + try { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const ids = await ToolRegistry.ids() + expect(ids).toContain("late") + }, + }) + expect( + install.mock.calls.some(([dir]) => path.normalize(dir) === path.normalize(path.join(tmp.path, ".opencode"))), + ).toBe(true) + } finally { + install.mockRestore() + } + }) + test("skips disabled tools before importing them", async () => { await using tmp = await tmpdir({ init: async (dir) => {