fix(core): restore plugins after failed activation

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<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* (
+    const load = Effect.fnUntraced(function* (plugin: Plugin) {
-      plugins: readonly { readonly plugin: Plugin; readonly version?: string }[],
+      const child = yield* Scope.fork(scope)
-    ) {
+      const inherit = yield* State.inherit()
-      const definitions = plugins.map((entry) => ({
+      const loaded = yield* Effect.suspend(() => plugin.effect(host)).pipe(
-        ...entry.plugin,
+        inherit,
-        id: ID.make(entry.plugin.id),
+        Effect.updateContext((_context: Context.Context<never>) => Context.make(Scope.Scope, child)),
-        ...(entry.version === undefined ? {} : { version: entry.version }),
+        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 previous = active.get(definition.id)
-                const loaded = yield* Effect.suspend(() => definition.effect(host)).pipe(
+                active.delete(definition.id)
-                  inherit,
+                if (previous) yield* Scope.close(previous.scope, Exit.void).pipe(Effect.ignore)
-                  Effect.updateContext((_context: Context.Context<never>) => Context.make(Scope.Scope, child)),
+
-                  Effect.withSpan("Plugin.load", { attributes: { "plugin.id": definition.id } }),
+                const loaded = yield* load(definition)
-                  Effect.andThen(events.publish(Event.Added, { id: definition.id })),
+                if (loaded) {
-                  Effect.onExit((exit) => (Exit.isFailure(exit) ? Scope.close(child, exit) : Effect.void)),
+                  active.set(definition.id, { plugin: definition, scope: loaded })
-                  Effect.exit,
-                )
-                if (Exit.isFailure(loaded)) {
-                  yield* Effect.logWarning("failed to load plugin", {
-                    "plugin.id": definition.id,
-                    cause: loaded.cause,
-                  })
                   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))
       }),
     )

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,
-          id: plugin.id,
+        effect: (host) => plugin.effect({ ...host, options: candidate.options }),
-          effect: (host) => plugin.effect({ ...host, options: candidate.options }),
+      } satisfies Plugin
-        } satisfies Plugin,
-        ...(candidate.mtime === undefined ? {} : { version: String(candidate.mtime) }),
-      }
     }).pipe(Effect.catchCause(() => Effect.succeed(undefined)))
   }).pipe(Effect.map((plugins) => plugins.filter((plugin) => plugin !== undefined)))
 })

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",

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() }])
+      yield* plugins.activate([managed()])
-      expect((yield* agents.get(AgentV2.ID.make("configured")))?.description).toBe("first")
-
-      yield* plugins.activate([{ plugin: managed(), version: "next" }])
       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 } },
+          { id: duplicate, effect: () => Effect.void },
-          { plugin: { 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 }])
+      yield* plugins.activate([good, bad])
-      expect(yield* plugins.list()).toEqual([
+      expect(yield* plugins.list()).toEqual([{ id: Plugin.ID.make("good") }, { id: Plugin.ID.make("bad") }])
-        { 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,
-            id,
+          effect: () => Effect.addFinalizer(() => Effect.sync(() => closed.push(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({

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.