fix(core): restore plugins after failed activation

This commit is contained in:
Dax Raad 2026-07-09 16:28:22 -04:00
parent bd0dffd781
commit 8fe78c8f8d
5 changed files with 141 additions and 85 deletions

View file

@ -21,7 +21,7 @@ import { ToolHooks } from "./tool/hooks"
import { PluginHooks } from "./plugin/hooks"
export interface Interface {
readonly activate: (plugins: readonly { readonly plugin: Plugin; readonly version?: string }[]) => Effect.Effect<void>
readonly activate: (plugins: readonly Plugin[]) => Effect.Effect<void>
readonly list: () => Effect.Effect<Info[]>
}
@ -32,72 +32,72 @@ const layer = Layer.effect(
Effect.gen(function* () {
const events = yield* EventV2.Service
const scope = yield* Scope.make()
const active = new Map<typeof ID.Type, Scope.Closeable>()
const active = new Map<typeof ID.Type, { readonly plugin: Plugin; readonly scope: Scope.Closeable }>()
const lock = Semaphore.makeUnsafe(1)
let generation: readonly { readonly id: typeof ID.Type; readonly version?: string }[] | undefined = []
let host: Parameters<Plugin["effect"]>[0]
const activate = Effect.fn("Plugin.activate")(function* (
plugins: readonly { readonly plugin: Plugin; readonly version?: string }[],
) {
const definitions = plugins.map((entry) => ({
...entry.plugin,
id: ID.make(entry.plugin.id),
...(entry.version === undefined ? {} : { version: entry.version }),
}))
const load = Effect.fnUntraced(function* (plugin: Plugin) {
const child = yield* Scope.fork(scope)
const inherit = yield* State.inherit()
const loaded = yield* Effect.suspend(() => plugin.effect(host)).pipe(
inherit,
Effect.updateContext((_context: Context.Context<never>) => Context.make(Scope.Scope, child)),
Effect.withSpan("Plugin.load", { attributes: { "plugin.id": plugin.id } }),
Effect.andThen(events.publish(Event.Added, { id: ID.make(plugin.id) })),
Effect.onExit((exit) => (Exit.isFailure(exit) ? Scope.close(child, exit) : Effect.void)),
Effect.exit,
)
if (Exit.isSuccess(loaded)) return child
yield* Effect.logWarning("failed to load plugin", {
"plugin.id": plugin.id,
cause: loaded.cause,
})
return undefined
})
const activate = Effect.fn("Plugin.activate")(function* (plugins: readonly Plugin[]) {
const definitions = plugins.map((plugin) => ({ ...plugin, id: ID.make(plugin.id) }))
const ids = new Set<typeof ID.Type>()
for (const definition of definitions) {
if (ids.has(definition.id)) return yield* Effect.die(new Error(`Duplicate plugin ID: ${definition.id}`))
if (ids.has(definition.id)) yield* Effect.die(new Error(`Duplicate plugin ID: ${definition.id}`))
ids.add(definition.id)
}
yield* lock.withPermit(
Effect.gen(function* () {
if (
generation !== undefined &&
generation.length === definitions.length &&
generation.every(
(plugin, index) => plugin.id === definitions[index]?.id && plugin.version === definitions[index]?.version,
) &&
definitions.every((definition) => active.has(definition.id))
) {
return
}
generation = undefined
yield* State.batch(
Effect.gen(function* () {
const scopes = Array.from(active.values()).toReversed()
active.clear()
const inherit = yield* State.inherit()
yield* Effect.forEach(scopes, (scope) => Scope.close(scope, Exit.void).pipe(Effect.ignore), {
discard: true,
})
for (const definition of definitions) {
const child = yield* Scope.fork(scope)
const loaded = yield* Effect.suspend(() => definition.effect(host)).pipe(
inherit,
Effect.updateContext((_context: Context.Context<never>) => Context.make(Scope.Scope, child)),
Effect.withSpan("Plugin.load", { attributes: { "plugin.id": definition.id } }),
Effect.andThen(events.publish(Event.Added, { id: definition.id })),
Effect.onExit((exit) => (Exit.isFailure(exit) ? Scope.close(child, exit) : Effect.void)),
Effect.exit,
)
if (Exit.isFailure(loaded)) {
yield* Effect.logWarning("failed to load plugin", {
"plugin.id": definition.id,
cause: loaded.cause,
})
const previous = active.get(definition.id)
active.delete(definition.id)
if (previous) yield* Scope.close(previous.scope, Exit.void).pipe(Effect.ignore)
const loaded = yield* load(definition)
if (loaded) {
active.set(definition.id, { plugin: definition, scope: loaded })
continue
}
active.set(definition.id, child)
if (!previous) continue
const restored = yield* load(previous.plugin)
if (restored) {
active.set(definition.id, { plugin: previous.plugin, scope: restored })
continue
}
yield* Effect.logError("failed to restore plugin; deactivating", {
"plugin.id": definition.id,
})
}
const removed = Array.from(active.entries())
.filter(([id]) => !ids.has(id))
.toReversed()
removed.forEach(([id]) => active.delete(id))
yield* Effect.forEach(removed, ([, entry]) => Scope.close(entry.scope, Exit.void).pipe(Effect.ignore), {
discard: true,
})
}),
)
generation = definitions.map((definition) => ({
id: definition.id,
...(definition.version === undefined ? {} : { version: definition.version }),
}))
yield* events.publish(Event.Updated, {})
}),
)
@ -106,7 +106,6 @@ const layer = Layer.effect(
yield* Effect.addFinalizer((exit) =>
Effect.gen(function* () {
active.clear()
generation = []
yield* State.batch(Scope.close(scope, exit))
}),
)

View file

@ -196,13 +196,13 @@ function apply(pre: readonly Plugin[], post: readonly Plugin[], operations: read
const load = Effect.fn("PluginSupervisor.load")(function* (plan: readonly Candidate[]) {
return yield* Effect.forEach(plan, (candidate) => {
if (candidate.type === "definition") return Effect.succeed({ plugin: candidate.definition })
if (candidate.type === "definition") return Effect.succeed(candidate.definition)
return Effect.gen(function* () {
const npm = yield* Npm.Service
const entrypoint = path.isAbsolute(candidate.specifier)
? pathToFileURL(candidate.specifier).href
: (yield* npm.add(candidate.specifier)).entrypoint
if (!entrypoint) return
if (!entrypoint) return undefined
// Bun currently ignores query parameters when caching file:// imports.
const source =
candidate.mtime === undefined
@ -213,12 +213,9 @@ const load = Effect.fn("PluginSupervisor.load")(function* (plan: readonly Candid
const value = (yield* Schema.decodeUnknownEffect(PluginModule)(mod)).default
const plugin = "effect" in value ? value : PluginPromise.fromPromise(value)
return {
plugin: {
id: plugin.id,
effect: (host) => plugin.effect({ ...host, options: candidate.options }),
} satisfies Plugin,
...(candidate.mtime === undefined ? {} : { version: String(candidate.mtime) }),
}
id: plugin.id,
effect: (host) => plugin.effect({ ...host, options: candidate.options }),
} satisfies Plugin
}).pipe(Effect.catchCause(() => Effect.succeed(undefined)))
}).pipe(Effect.map((plugins) => plugins.filter((plugin) => plugin !== undefined)))
})

View file

@ -4,7 +4,7 @@ import { describe, expect } from "bun:test"
import { Config } from "@opencode-ai/schema/config"
import { Plugin } from "@opencode-ai/schema/plugin"
import { Money } from "@opencode-ai/schema/money"
import { Context, DateTime, Deferred, Effect, Equal, Fiber, Hash, RcMap, Schema, Stream } from "effect"
import { DateTime, Deferred, Effect, Equal, Fiber, Hash, RcMap, Schema, Stream } from "effect"
import { Plugin as EffectPlugin } from "@opencode-ai/plugin/v2/effect"
import { AgentV2 } from "@opencode-ai/core/agent"
import { Catalog } from "@opencode-ai/core/catalog"
@ -706,7 +706,7 @@ describe("LocationServiceMap", () => {
})
.pipe(Effect.asVoid),
})
yield* plugins.activate([{ plugin: reviewer }])
yield* plugins.activate([reviewer])
expect(yield* (yield* AgentV2.Service).get(AgentV2.ID.make("reviewer"))).toMatchObject({
description: "Reviews code",

View file

@ -38,7 +38,7 @@ describe("PluginV2", () => {
}),
)
it.effect("skips identical generations and replaces changed plugin versions", () =>
it.effect("replaces plugins by ID", () =>
Effect.gen(function* () {
const plugins = yield* PluginV2.Service
const agents = yield* AgentV2.Service
@ -61,15 +61,12 @@ describe("PluginV2", () => {
.pipe(Effect.asVoid),
})
yield* plugins.activate([{ plugin: managed() }])
yield* plugins.activate([managed()])
expect((yield* agents.get(AgentV2.ID.make("configured")))?.description).toBe("first")
description = "second"
yield* plugins.activate([{ plugin: managed() }])
expect((yield* agents.get(AgentV2.ID.make("configured")))?.description).toBe("first")
yield* plugins.activate([{ plugin: managed(), version: "next" }])
yield* plugins.activate([managed()])
expect((yield* agents.get(AgentV2.ID.make("configured")))?.description).toBe("second")
expect(yield* Fiber.join(updated)).toHaveLength(2)
@ -78,17 +75,17 @@ describe("PluginV2", () => {
}),
)
it.effect("rejects duplicate IDs before replacing the active generation", () =>
it.effect("rejects duplicate IDs before replacing active plugins", () =>
Effect.gen(function* () {
const plugins = yield* PluginV2.Service
const active = Plugin.ID.make("active")
const duplicate = "duplicate"
yield* plugins.activate([{ plugin: { id: active, effect: () => Effect.void } }])
yield* plugins.activate([{ id: active, effect: () => Effect.void }])
const result = yield* plugins
.activate([
{ plugin: { id: duplicate, effect: () => Effect.void } },
{ plugin: { id: duplicate, effect: () => Effect.void } },
{ id: duplicate, effect: () => Effect.void },
{ id: duplicate, effect: () => Effect.void },
])
.pipe(Effect.exit)
@ -121,16 +118,81 @@ describe("PluginV2", () => {
},
})
yield* plugins.activate([{ plugin: good }, { plugin: bad }])
yield* plugins.activate([good, bad])
expect(yield* plugins.list()).toEqual([{ id: Plugin.ID.make("good") }])
expect((yield* agents.get(AgentV2.ID.make("configured")))?.description).toBe("loaded")
fail = false
yield* plugins.activate([{ plugin: good }, { plugin: bad }])
expect(yield* plugins.list()).toEqual([
{ id: Plugin.ID.make("good") },
{ id: Plugin.ID.make("bad") },
])
yield* plugins.activate([good, bad])
expect(yield* plugins.list()).toEqual([{ id: Plugin.ID.make("good") }, { id: Plugin.ID.make("bad") }])
}),
)
it.effect("restores the previous plugin when its replacement fails", () =>
Effect.gen(function* () {
const plugins = yield* PluginV2.Service
const agents = yield* AgentV2.Service
const previous = EffectPlugin.define({
id: "managed",
effect: (ctx) =>
ctx.agent
.transform((agents) =>
agents.update("configured", (agent) => {
agent.description = "previous"
}),
)
.pipe(Effect.asVoid),
})
const replacement = EffectPlugin.define({
id: "managed",
effect: (ctx) =>
Effect.gen(function* () {
yield* ctx.agent.transform((agents) =>
agents.update("configured", (agent) => {
agent.description = "replacement"
}),
)
return yield* Effect.die(new Error("replacement failed"))
}),
})
yield* plugins.activate([previous])
yield* plugins.activate([replacement])
expect(yield* plugins.list()).toEqual([{ id: Plugin.ID.make("managed") }])
expect((yield* agents.get(AgentV2.ID.make("configured")))?.description).toBe("previous")
}),
)
it.effect("deactivates a plugin when replacement and restoration fail", () =>
Effect.gen(function* () {
const plugins = yield* PluginV2.Service
const agents = yield* AgentV2.Service
let loads = 0
const previous = EffectPlugin.define({
id: "managed",
effect: (ctx) => {
loads++
if (loads > 1) return Effect.die(new Error("restoration failed"))
return ctx.agent
.transform((agents) =>
agents.update("configured", (agent) => {
agent.description = "previous"
}),
)
.pipe(Effect.asVoid)
},
})
const replacement = EffectPlugin.define({
id: "managed",
effect: () => Effect.die(new Error("replacement failed")),
})
yield* plugins.activate([previous])
yield* plugins.activate([replacement])
expect(yield* plugins.list()).toEqual([])
expect(yield* agents.get(AgentV2.ID.make("configured"))).toBeUndefined()
}),
)
@ -140,10 +202,8 @@ describe("PluginV2", () => {
const closed: string[] = []
yield* plugins.activate(
["first", "second"].map((id) => ({
plugin: {
id,
effect: () => Effect.addFinalizer(() => Effect.sync(() => closed.push(id))),
},
id,
effect: () => Effect.addFinalizer(() => Effect.sync(() => closed.push(id))),
})),
)
@ -166,7 +226,7 @@ describe("PluginV2", () => {
),
})
yield* plugins.activate([{ plugin }]).pipe(Effect.provideService(Secret, "secret"))
yield* plugins.activate([plugin]).pipe(Effect.provideService(Secret, "secret"))
expect(visible).toBe(false)
}),
@ -194,7 +254,7 @@ describe("PluginV2", () => {
.pipe(Effect.orDie),
})
yield* plugins.activate([{ plugin }])
yield* plugins.activate([plugin])
expect((yield* registry.materialize({ model: testModel })).definitions.map((tool) => tool.name)).toContain(
"plugin_tool",
)
@ -229,7 +289,7 @@ describe("PluginV2", () => {
.pipe(Effect.orDie),
})
yield* plugins.activate([{ plugin }])
yield* plugins.activate([plugin])
expect((yield* registry.materialize({ model: testModel })).definitions.map((tool) => tool.name)).toEqual([
"plain",
@ -288,7 +348,7 @@ describe("PluginV2", () => {
}),
})
yield* plugins.activate([{ plugin }])
yield* plugins.activate([plugin])
const materialized = yield* registry.materialize({ model: testModel })
const settlement = yield* materialized.settle({

View file

@ -290,7 +290,7 @@ begin batch
→ end batch
```
Registration itself is not staged per plugin. If setup fails, closing the plugin's child scope removes every registration made before the failure.
Registration itself is not staged per plugin. If setup fails, closing the plugin's child scope removes every registration made before the failure. A replacement then retries the previous definition; if that setup also fails, the plugin remains inactive.
Outside a batch, transform registration and disposal rebuild immediately.