Sfoglia il codice sorgente

fix(core): skip unchanged plugin activation

Dax Raad 1 mese fa
parent
commit
a4a948316b

+ 22 - 4
packages/core/src/plugin.ts

@@ -21,10 +21,14 @@ import { ToolHooks } from "./tool/hooks"
 import { PluginHooks } from "./plugin/hooks"
 
 export interface Interface {
-  readonly activate: (plugins: readonly Plugin[]) => Effect.Effect<void>
+  readonly activate: (plugins: readonly Versioned[]) => Effect.Effect<void>
   readonly list: () => Effect.Effect<Info[]>
 }
 
+export interface Versioned extends Plugin {
+  readonly version: string
+}
+
 export class Service extends Context.Service<Service, Interface>()("@opencode/v2/Plugin") {}
 
 const layer = Layer.effect(
@@ -32,11 +36,11 @@ const layer = Layer.effect(
   Effect.gen(function* () {
     const events = yield* EventV2.Service
     const scope = yield* Scope.make()
-    const active = new Map<typeof ID.Type, { readonly plugin: Plugin; readonly scope: Scope.Closeable }>()
+    const active = new Map<typeof ID.Type, { readonly plugin: Versioned; readonly scope: Scope.Closeable }>()
     const lock = Semaphore.makeUnsafe(1)
     let host: Parameters<Plugin["effect"]>[0]
 
-    const load = Effect.fnUntraced(function* (plugin: Plugin) {
+    const load = Effect.fnUntraced(function* (plugin: Versioned) {
       const child = yield* Scope.fork(scope)
       const inherit = yield* State.inherit()
       const loaded = yield* Effect.suspend(() => plugin.effect(host)).pipe(
@@ -55,7 +59,7 @@ const layer = Layer.effect(
       return undefined
     })
 
-    const activate = Effect.fn("Plugin.activate")(function* (plugins: readonly Plugin[]) {
+    const activate = Effect.fn("Plugin.activate")(function* (plugins: readonly Versioned[]) {
       const definitions = plugins.map((plugin) => ({ ...plugin, id: ID.make(plugin.id) }))
       const ids = new Set<typeof ID.Type>()
       for (const definition of definitions) {
@@ -65,6 +69,20 @@ const layer = Layer.effect(
 
       yield* lock.withPermit(
         Effect.gen(function* () {
+          const next = definitions.map((definition) => ({ id: definition.id, version: definition.version }))
+          const current = Array.from(active.values(), (entry) => ({
+            id: entry.plugin.id,
+            version: entry.plugin.version,
+          }))
+          if (
+            current.length === next.length &&
+            current.every((definition, index) => {
+              const candidate = next[index]
+              return definition.id === candidate?.id && definition.version === candidate.version
+            })
+          )
+            return
+
           yield* State.batch(
             Effect.gen(function* () {
               for (const definition of definitions) {

+ 5 - 3
packages/core/src/plugin/sdk.ts

@@ -4,6 +4,7 @@ import type { Plugin } from "@opencode-ai/plugin/v2/effect/plugin"
 import { Context, Effect, Layer } from "effect"
 import { makeGlobalNode } from "../effect/app-node"
 import { EventV2 } from "../event"
+import type { PluginV2 } from "../plugin"
 
 export const Updated = EventV2.ephemeral({ type: "sdk.plugin.updated", schema: {} })
 
@@ -20,7 +21,7 @@ export const Updated = EventV2.ephemeral({ type: "sdk.plugin.updated", schema: {
  */
 export interface Interface {
   readonly register: (plugin: Plugin) => Effect.Effect<void>
-  readonly all: () => readonly Plugin[]
+  readonly all: () => readonly PluginV2.Versioned[]
 }
 
 export class Service extends Context.Service<Service, Interface>()("@opencode/SdkPlugins") {}
@@ -29,11 +30,12 @@ export const layer = Layer.effect(
   Service,
   Effect.gen(function* () {
     const events = yield* EventV2.Service
-    const plugins = new Map<string, Plugin>()
+    const plugins = new Map<string, PluginV2.Versioned>()
+    let revision = 0
     return Service.of({
       register: (plugin) =>
         Effect.sync(() => {
-          plugins.set(plugin.id, plugin)
+          plugins.set(plugin.id, { ...plugin, version: String(++revision) })
         }).pipe(Effect.andThen(events.publish(Updated, {})), Effect.asVoid),
       all: () => [...plugins.values()],
     })

+ 8 - 6
packages/core/src/plugin/supervisor.ts

@@ -116,15 +116,15 @@ const scan = Effect.fn("PluginSupervisor.scan")(function* (entries: readonly Con
 })
 
 const resolve = Effect.fn("PluginSupervisor.resolve")(function* (
-  pre: readonly Plugin[],
-  post: readonly Plugin[],
+  pre: readonly PluginV2.Versioned[],
+  post: readonly PluginV2.Versioned[],
   operations: readonly Operation[],
 ) {
   const matches = (selector: string, target: string) =>
     selector === "*" || (selector.endsWith(".*") ? target.startsWith(selector.slice(0, -1)) : selector === target)
   const definitions = [...pre, ...post]
   const enabled = new Set(definitions.map((plugin) => plugin.id))
-  const packages = new Map<string, Plugin>()
+  const packages = new Map<string, PluginV2.Versioned>()
   const plugins = () => [...definitions, ...packages.values()]
 
   for (const operation of operations) {
@@ -178,8 +178,9 @@ const load = Effect.fn("PluginSupervisor.load")(function* (operation: Extract<Op
   const plugin = "effect" in value ? value : PluginPromise.fromPromise(value)
   return {
     id: plugin.id,
+    version: JSON.stringify(operation),
     effect: (host) => plugin.effect({ ...host, options: operation.options }),
-  } satisfies Plugin
+  } satisfies PluginV2.Versioned
 })
 
 function discoverDirectory(fs: FSUtil.Interface, directory: string) {
@@ -253,10 +254,11 @@ const layer = Layer.effect(
           // Resolve OpenCode's internal plugins with their privileged Location services.
           const internal = yield* PluginInternal.list()
           // Combine internal plugins with host-contributed SDK plugins in boot order.
-          const pre = [...internal.pre, ...sdk.all()]
+          const pre = [...internal.pre.map((plugin) => ({ ...plugin, version: "internal" })), ...sdk.all()]
+          const post = internal.post.map((plugin) => ({ ...plugin, version: "internal" }))
           const operations = yield* scan(yield* config.entries())
           // Apply config operations and load enabled package plugins into one ordered generation.
-          const plugins = yield* resolve(pre, internal.post, operations)
+          const plugins = yield* resolve(pre, post, operations)
           // Replace the active generation in one scoped, batched activation.
           yield* registry.activate(plugins)
           applied = target

+ 31 - 1
packages/core/test/location-layer.test.ts

@@ -249,6 +249,36 @@ describe("LocationServiceMap", () => {
     ),
   )
 
+  itWithSdk.live("does not reload plugins when config updates leave plugin operations unchanged", () =>
+    Effect.acquireRelease(
+      Effect.promise(() => tmpdir()),
+      (dir) => Effect.promise(() => dir[Symbol.asyncDispose]()),
+    ).pipe(
+      Effect.flatMap((dir) =>
+        Effect.gen(function* () {
+          const activations = { count: 0 }
+          const sdk = yield* SdkPlugins.Service
+          yield* sdk.register(
+            EffectPlugin.define({
+              id: "unchanged-config-plugin",
+              effect: () => Effect.sync(() => ++activations.count).pipe(Effect.asVoid),
+            }),
+          )
+
+          const locations = yield* LocationServiceMap.Service
+          const context = yield* locations.contextEffect(Location.Ref.make({ directory: AbsolutePath.make(dir.path) }))
+          yield* PluginSupervisor.Service.use((supervisor) => supervisor.flush).pipe(Effect.provide(context))
+          expect(activations.count).toBe(1)
+
+          yield* EventV2.Service.use((events) => events.publish(Config.Event.Updated, {})).pipe(Effect.provide(context))
+          yield* Effect.sleep("200 millis")
+
+          expect(activations.count).toBe(1)
+        }),
+      ),
+    ),
+  )
+
   itWithSdk.live("keeps flush open while later hot reload runs", () =>
     Effect.acquireRelease(
       Effect.promise(() => tmpdir()),
@@ -706,7 +736,7 @@ describe("LocationServiceMap", () => {
                 })
                 .pipe(Effect.asVoid),
           })
-          yield* plugins.activate([reviewer])
+          yield* plugins.activate([{ ...reviewer, version: "1" }])
 
           expect(yield* (yield* AgentV2.Service).get(AgentV2.ID.make("reviewer"))).toMatchObject({
             description: "Reviews code",

+ 32 - 20
packages/core/test/plugin.test.ts

@@ -18,6 +18,8 @@ const it = testEffect(PluginTestLayer)
 
 class Secret extends Context.Service<Secret, string>()("@opencode/test/PluginSecret") {}
 
+const versioned = <R>(plugin: EffectPlugin.Plugin<R>, version = "1") => ({ ...plugin, version })
+
 describe("PluginV2", () => {
   it.live("exposes public events through the plugin context", () =>
     Effect.gen(function* () {
@@ -37,15 +39,18 @@ describe("PluginV2", () => {
     }),
   )
 
-  it.effect("replaces plugins by ID", () =>
+  it.effect("replaces plugins by ID and version", () =>
     Effect.gen(function* () {
       const plugins = yield* PluginV2.Service
       const agents = yield* AgentV2.Service
       const events = yield* EventV2.Service
       let description = "first"
-      const updated = yield* events
-        .subscribe(Plugin.Event.Updated)
-        .pipe(Stream.take(2), Stream.runCollect, Effect.forkScoped({ startImmediately: true }))
+      let updates = 0
+      const unsubscribe = yield* events.listen((event) =>
+        Effect.sync(() => {
+          if (event.type === Plugin.Event.Updated.type) updates++
+        }),
+      )
 
       const managed = () =>
         EffectPlugin.define({
@@ -60,17 +65,23 @@ describe("PluginV2", () => {
               .pipe(Effect.asVoid),
         })
 
-      yield* plugins.activate([managed()])
+      yield* plugins.activate([versioned(managed(), "1")])
 
       expect((yield* agents.get(AgentV2.ID.make("configured")))?.description).toBe("first")
 
       description = "second"
-      yield* plugins.activate([managed()])
+      yield* plugins.activate([versioned(managed(), "2")])
+      expect((yield* agents.get(AgentV2.ID.make("configured")))?.description).toBe("second")
+
+      description = "third"
+      yield* plugins.activate([versioned(managed(), "2")])
+      expect(updates).toBe(2)
       expect((yield* agents.get(AgentV2.ID.make("configured")))?.description).toBe("second")
-      expect(yield* Fiber.join(updated)).toHaveLength(2)
 
       yield* plugins.activate([])
       expect(yield* agents.get(AgentV2.ID.make("configured"))).toBeUndefined()
+      expect(updates).toBe(3)
+      yield* unsubscribe
     }),
   )
 
@@ -79,12 +90,12 @@ describe("PluginV2", () => {
       const plugins = yield* PluginV2.Service
       const active = Plugin.ID.make("active")
       const duplicate = "duplicate"
-      yield* plugins.activate([{ id: active, effect: () => Effect.void }])
+      yield* plugins.activate([{ id: active, version: "1", effect: () => Effect.void }])
 
       const result = yield* plugins
         .activate([
-          { id: duplicate, effect: () => Effect.void },
-          { id: duplicate, effect: () => Effect.void },
+          { id: duplicate, version: "1", effect: () => Effect.void },
+          { id: duplicate, version: "1", effect: () => Effect.void },
         ])
         .pipe(Effect.exit)
 
@@ -117,12 +128,12 @@ describe("PluginV2", () => {
         },
       })
 
-      yield* plugins.activate([good, bad])
+      yield* plugins.activate([versioned(good), versioned(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([good, bad])
+      yield* plugins.activate([versioned(good), versioned(bad, "2")])
       expect(yield* plugins.list()).toEqual([{ id: Plugin.ID.make("good") }, { id: Plugin.ID.make("bad") }])
     }),
   )
@@ -155,8 +166,8 @@ describe("PluginV2", () => {
           }),
       })
 
-      yield* plugins.activate([previous])
-      yield* plugins.activate([replacement])
+      yield* plugins.activate([versioned(previous)])
+      yield* plugins.activate([versioned(replacement, "2")])
 
       expect(yield* plugins.list()).toEqual([{ id: Plugin.ID.make("managed") }])
       expect((yield* agents.get(AgentV2.ID.make("configured")))?.description).toBe("previous")
@@ -187,8 +198,8 @@ describe("PluginV2", () => {
         effect: () => Effect.die(new Error("replacement failed")),
       })
 
-      yield* plugins.activate([previous])
-      yield* plugins.activate([replacement])
+      yield* plugins.activate([versioned(previous)])
+      yield* plugins.activate([versioned(replacement, "2")])
 
       expect(yield* plugins.list()).toEqual([])
       expect(yield* agents.get(AgentV2.ID.make("configured"))).toBeUndefined()
@@ -202,6 +213,7 @@ describe("PluginV2", () => {
       yield* plugins.activate(
         ["first", "second"].map((id) => ({
           id,
+          version: "1",
           effect: () => Effect.addFinalizer(() => Effect.sync(() => closed.push(id))),
         })),
       )
@@ -225,7 +237,7 @@ describe("PluginV2", () => {
           ),
       })
 
-      yield* plugins.activate([plugin]).pipe(Effect.provideService(Secret, "secret"))
+      yield* plugins.activate([versioned(plugin)]).pipe(Effect.provideService(Secret, "secret"))
 
       expect(visible).toBe(false)
     }),
@@ -253,7 +265,7 @@ describe("PluginV2", () => {
             .pipe(Effect.orDie),
       })
 
-      yield* plugins.activate([plugin])
+      yield* plugins.activate([versioned(plugin)])
       expect((yield* registry.materialize()).definitions.map((tool) => tool.name)).toContain("plugin_tool")
 
       yield* plugins.activate([])
@@ -284,7 +296,7 @@ describe("PluginV2", () => {
             .pipe(Effect.orDie),
       })
 
-      yield* plugins.activate([plugin])
+      yield* plugins.activate([versioned(plugin)])
 
       expect((yield* registry.materialize()).definitions.map((tool) => tool.name)).toEqual([
         "plain",
@@ -343,7 +355,7 @@ describe("PluginV2", () => {
           }),
       })
 
-      yield* plugins.activate([plugin])
+      yield* plugins.activate([versioned(plugin)])
 
       const materialized = yield* registry.materialize()
       const settlement = yield* materialized.settle({