From a4a948316bf2c7179f7ae7f4a2956fbe6ddb627b Mon Sep 17 00:00:00 2001 From: Dax Raad Date: Fri, 10 Jul 2026 20:49:16 -0400 Subject: [PATCH] fix(core): skip unchanged plugin activation --- packages/core/src/plugin.ts | 26 ++++++++++-- packages/core/src/plugin/sdk.ts | 8 ++-- packages/core/src/plugin/supervisor.ts | 14 +++--- packages/core/test/location-layer.test.ts | 32 +++++++++++++- packages/core/test/plugin.test.ts | 52 ++++++++++++++--------- 5 files changed, 98 insertions(+), 34 deletions(-) diff --git a/packages/core/src/plugin.ts b/packages/core/src/plugin.ts index 610d14e3c3..4054873851 100644 --- a/packages/core/src/plugin.ts +++ b/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 + readonly activate: (plugins: readonly Versioned[]) => Effect.Effect readonly list: () => Effect.Effect } +export interface Versioned extends Plugin { + readonly version: string +} + export class Service extends Context.Service()("@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() + const active = new Map() const lock = Semaphore.makeUnsafe(1) let host: Parameters[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() 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) { diff --git a/packages/core/src/plugin/sdk.ts b/packages/core/src/plugin/sdk.ts index 3dd42a8037..56df0813dd 100644 --- a/packages/core/src/plugin/sdk.ts +++ b/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 - readonly all: () => readonly Plugin[] + readonly all: () => readonly PluginV2.Versioned[] } export class Service extends Context.Service()("@opencode/SdkPlugins") {} @@ -29,11 +30,12 @@ export const layer = Layer.effect( Service, Effect.gen(function* () { const events = yield* EventV2.Service - const plugins = new Map() + const plugins = new Map() + 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()], }) diff --git a/packages/core/src/plugin/supervisor.ts b/packages/core/src/plugin/supervisor.ts index 9b056c994e..c29c6a8647 100644 --- a/packages/core/src/plugin/supervisor.ts +++ b/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() + const packages = new Map() const plugins = () => [...definitions, ...packages.values()] for (const operation of operations) { @@ -178,8 +178,9 @@ const load = Effect.fn("PluginSupervisor.load")(function* (operation: Extract 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 diff --git a/packages/core/test/location-layer.test.ts b/packages/core/test/location-layer.test.ts index d4ae3b7cf5..ec84b3b8ad 100644 --- a/packages/core/test/location-layer.test.ts +++ b/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", diff --git a/packages/core/test/plugin.test.ts b/packages/core/test/plugin.test.ts index a21dc25479..8c2db479e5 100644 --- a/packages/core/test/plugin.test.ts +++ b/packages/core/test/plugin.test.ts @@ -18,6 +18,8 @@ const it = testEffect(PluginTestLayer) class Secret extends Context.Service()("@opencode/test/PluginSecret") {} +const versioned = (plugin: EffectPlugin.Plugin, 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({