Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions packages/opencode/src/plugin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,30 @@ function getLegacyPlugins(mod: Record<string, unknown>) {
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(
Expand Down Expand Up @@ -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 ?? [])
Expand Down Expand Up @@ -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
Expand Down
98 changes: 98 additions & 0 deletions packages/opencode/test/plugin/falsy-hooks.test.ts
Original file line number Diff line number Diff line change
@@ -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<A, E, R>(source: string, self: Effect.Effect<A, E, R>) {
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)
}),
),
)
})
Loading