From bd61dbfc72ae60a5a1cf10d1348928c3d217f809 Mon Sep 17 00:00:00 2001 From: CreatorGhost Date: Sun, 12 Jul 2026 00:53:23 +0530 Subject: [PATCH] fix(opencode): skip falsy plugin hook returns instead of crashing provider listing Ported from upstream anomalyco/opencode#36102. --- packages/opencode/src/plugin/index.ts | 31 +++++- .../opencode/test/plugin/falsy-hooks.test.ts | 98 +++++++++++++++++++ 2 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 packages/opencode/test/plugin/falsy-hooks.test.ts diff --git a/packages/opencode/src/plugin/index.ts b/packages/opencode/src/plugin/index.ts index 1558c707c3f7..8ae7d3f31b31 100644 --- a/packages/opencode/src/plugin/index.ts +++ b/packages/opencode/src/plugin/index.ts @@ -107,17 +107,30 @@ function getLegacyPlugins(mod: Record) { return result } +// Returns the specs of plugin entries that resolved to no hooks so the caller +// can surface a warning. A falsy hooks entry must never reach the hooks array: +// consumers dereference entries directly (e.g. hook.provider in the Provider +// state build), so one undefined entry breaks provider listing for the whole +// instance. Legacy modules are the common source — every function export is +// loaded as a plugin, so a stray helper export lands here with an undefined +// return value. async function applyPlugin(load: PluginLoader.Loaded, input: PluginInput, hooks: Hooks[]) { + const skipped: string[] = [] const plugin = readV1Plugin(load.mod, load.spec, "server", "detect") if (plugin) { await resolvePluginId(load.source, load.spec, load.target, readPluginId(plugin.id, load.spec), load.pkg) - hooks.push(await (plugin as PluginModule).server(input, load.options)) - return + const init = await (plugin as PluginModule).server(input, load.options) + if (init) hooks.push(init) + else skipped.push(load.spec) + return skipped } for (const server of getLegacyPlugins(load.mod)) { - hooks.push(await server(input, load.options)) + const init = await server(input, load.options) + if (init) hooks.push(init) + else skipped.push(load.spec) } + return skipped } const layer = Layer.effect( @@ -171,7 +184,10 @@ const layer = Layer.effect( Effect.tapError((error) => Effect.logError("failed to load internal plugin", { name: plugin.name, error })), Effect.option, ) - if (init._tag === "Some") hooks.push(init.value) + if (init._tag === "Some") { + if (init.value) hooks.push(init.value) + else yield* Effect.logWarning("internal plugin returned no hooks", { name: plugin.name }) + } } const plugins = flags.pure ? [] : (cfg.plugin_origins ?? []) @@ -224,6 +240,13 @@ const layer = Layer.effect( return message }, }).pipe( + Effect.tap((skipped) => + skipped.length + ? Effect.logWarning("plugin returned no hooks — export a Plugin function or a PluginModule default", { + path: load.spec, + }) + : Effect.void, + ), Effect.tapError((error) => Effect.logError("failed to load plugin", { path: load.spec, error })), Effect.catch(() => { // TODO: make proper events for this diff --git a/packages/opencode/test/plugin/falsy-hooks.test.ts b/packages/opencode/test/plugin/falsy-hooks.test.ts new file mode 100644 index 000000000000..fc66af1cf685 --- /dev/null +++ b/packages/opencode/test/plugin/falsy-hooks.test.ts @@ -0,0 +1,98 @@ +import { describe, expect } from "bun:test" +import { Effect } from "effect" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { Npm } from "@opencode-ai/core/npm" +import path from "path" +import { pathToFileURL } from "url" +import { Account } from "../../src/account/account" +import { Auth } from "../../src/auth" +import { RuntimeFlags } from "../../src/effect/runtime-flags" +import { Plugin } from "../../src/plugin/index" + +import { TestInstance } from "../fixture/fixture" +import { testEffect } from "../lib/effect" +import { AccountTest } from "../fake/account" +import { AuthTest } from "../fake/auth" +import { NpmTest } from "../fake/npm" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" + +const it = testEffect( + AppNodeBuilder.build(LayerNode.group([Plugin.node, CrossSpawnSpawner.node]), [ + [Auth.node, AuthTest.empty], + [Account.node, AccountTest.empty], + [Npm.node, NpmTest.noop], + [RuntimeFlags.node, RuntimeFlags.layer({ disableDefaultPlugins: true })], + ]), +) + +function withProject(source: string, self: Effect.Effect) { + return Effect.gen(function* () { + const test = yield* TestInstance + const file = path.join(test.directory, "plugin.ts") + yield* Effect.all( + [ + Effect.promise(() => Bun.write(file, source)), + Effect.promise(() => + Bun.write( + path.join(test.directory, "opencode.json"), + JSON.stringify( + { + $schema: "https://opencode.ai/config.json", + plugin: [pathToFileURL(file).href], + }, + null, + 2, + ), + ), + ), + ], + { discard: true, concurrency: 2 }, + ) + return yield* self + }) +} + +const listHooks = Effect.gen(function* () { + const plugin = yield* Plugin.Service + yield* plugin.init() + return yield* plugin.list() +}) + +// A falsy entry in the hooks array breaks every consumer that dereferences +// hook properties directly — most visibly the Provider state build +// (hook.provider), which turns into a 500 from /config/providers and makes +// the whole instance unusable. These tests pin down that plugin loading +// never lets a falsy hook through. +describe("plugin falsy hook returns", () => { + it.instance("legacy module: stray helper export resolving undefined is skipped, real plugin loads", () => + withProject( + [ + "// legacy shape: every function export is loaded as a plugin, so the", + "// helper below is called with (input, options) and returns undefined.", + "export function helper() {}", + "export const realPlugin = async () => ({", + " async config(_cfg) {},", + "})", + "export default realPlugin", + "", + ].join("\n"), + Effect.gen(function* () { + const hooks = yield* listHooks + expect(hooks.every(Boolean)).toBe(true) + // the real plugin's hooks made it in; only the helper was dropped + expect(hooks.some((hook) => typeof hook.config === "function")).toBe(true) + }), + ), + ) + + it.instance("v1 module: server() resolving undefined is skipped", () => + withProject( + ["export default {", ' id: "demo.falsy-hooks",', " server: async () => undefined,", "}", ""].join("\n"), + Effect.gen(function* () { + const hooks = yield* listHooks + expect(hooks.every(Boolean)).toBe(true) + }), + ), + ) +})