diff --git a/packages/core/src/plugin.ts b/packages/core/src/plugin.ts index 4e0858fd16..610d14e3c3 100644 --- a/packages/core/src/plugin.ts +++ b/packages/core/src/plugin.ts @@ -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 + readonly activate: (plugins: readonly Plugin[]) => Effect.Effect readonly list: () => Effect.Effect } @@ -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() + const active = new Map() const lock = Semaphore.makeUnsafe(1) - let generation: readonly { readonly id: typeof ID.Type; readonly version?: string }[] | undefined = [] let host: Parameters[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) => 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() 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) => 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)) }), ) diff --git a/packages/core/src/plugin/supervisor.ts b/packages/core/src/plugin/supervisor.ts index 0dd3a31d72..1bc240bf78 100644 --- a/packages/core/src/plugin/supervisor.ts +++ b/packages/core/src/plugin/supervisor.ts @@ -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))) }) diff --git a/packages/core/test/location-layer.test.ts b/packages/core/test/location-layer.test.ts index 34288c042c..d4ae3b7cf5 100644 --- a/packages/core/test/location-layer.test.ts +++ b/packages/core/test/location-layer.test.ts @@ -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", diff --git a/packages/core/test/plugin.test.ts b/packages/core/test/plugin.test.ts index 79fcc5d0f6..5b91a336ad 100644 --- a/packages/core/test/plugin.test.ts +++ b/packages/core/test/plugin.test.ts @@ -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({ diff --git a/packages/plugin/src/v2/effect/PLAN.md b/packages/plugin/src/v2/effect/PLAN.md index 284d1ffe09..12cac7deea 100644 --- a/packages/plugin/src/v2/effect/PLAN.md +++ b/packages/plugin/src/v2/effect/PLAN.md @@ -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.