From 17f6cd601e39fc34e8e032916b5ed7357aa26f33 Mon Sep 17 00:00:00 2001 From: thdxr <826656+thdxr@users.noreply.github.com> Date: Mon, 14 Sep 2026 00:23:39 +0000 Subject: [PATCH] refactor(core): unify compatibility skill loading --- packages/core/src/config.ts | 25 +++--- .../core/src/config/plugin/compatibility.ts | 78 ------------------- packages/core/src/config/plugin/skill.ts | 20 ++++- packages/core/src/plugin/internal.ts | 2 - packages/core/test/config/config.test.ts | 28 +++++++ packages/core/test/config/skill.test.ts | 24 +++--- 6 files changed, 69 insertions(+), 108 deletions(-) delete mode 100644 packages/core/src/config/plugin/compatibility.ts diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index a56c1119f91b..a799a1ea5252 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -27,7 +27,7 @@ export function latest(entries: readonly Entry[], key: K): export interface Interface { /** Returns location config documents and discovery sources from lowest to highest priority. */ readonly entries: () => Effect.Effect - /** Compatibility roots consumed by internal compatibility plugins. */ + /** Compatibility roots consumed by the skill config plugin. */ readonly compatibility?: () => Effect.Effect<{ readonly claude: readonly AbsolutePath[] readonly agents: readonly AbsolutePath[] @@ -237,8 +237,15 @@ export const layer = (options?: Options) => ] }) + const loadCompatibility = Effect.fnUntraced(function* (sources: ConfigDiscovery.Sources) { + return yield* Effect.all({ + claude: Effect.filter(sources.claude, fs.isDir), + agents: Effect.filter(sources.agents, fs.isDir), + }) + }) + const initial = yield* ConfigDiscovery.discover(options) - let sources = initial + let compatibility = yield* loadCompatibility(initial) let configs = yield* load(initial) const updates = yield* PubSub.unbounded() const reloads = yield* PubSub.sliding(1) @@ -266,12 +273,10 @@ export const layer = (options?: Options) => function* () { const discovered = yield* ConfigDiscovery.discover(options) const next = yield* load(discovered) + const nextCompatibility = yield* loadCompatibility(discovered) yield* reconcile(discovered) - const compatibilityChanged = - !isDeepStrictEqual(sources.claude, discovered.claude) || - !isDeepStrictEqual(sources.agents, discovered.agents) - if (isDeepStrictEqual(configs, next) && !compatibilityChanged) return - sources = discovered + if (isDeepStrictEqual(configs, next) && isDeepStrictEqual(compatibility, nextCompatibility)) return + compatibility = nextCompatibility configs = next yield* bus.publish(Event.Updated, {}) }, @@ -352,11 +357,7 @@ export const layer = (options?: Options) => entries: Effect.fnUntraced(function* () { return configs }), - compatibility: () => - Effect.all({ - claude: Effect.filter(sources.claude, fs.isDir), - agents: Effect.filter(sources.agents, fs.isDir), - }), + compatibility: () => Effect.succeed(compatibility), changes: () => Stream.fromPubSub(updates), update, }) diff --git a/packages/core/src/config/plugin/compatibility.ts b/packages/core/src/config/plugin/compatibility.ts deleted file mode 100644 index c25cbce42dfa..000000000000 --- a/packages/core/src/config/plugin/compatibility.ts +++ /dev/null @@ -1,78 +0,0 @@ -export * as ConfigCompatibilityPlugin from "./compatibility.js" - -import { define } from "@opencode/plugin/effect/plugin" -import { FSUtil } from "@opencode/util/fs-util" -import path from "path" -import { Effect, FiberMap, PubSub, Semaphore, Stream } from "effect" -import { Config } from "../../config.js" -import { Watcher } from "../../filesystem/watcher.js" -import { AbsolutePath } from "../../schema.js" -import { Skill } from "../../skill.js" -import { SkillFile } from "./skill-file.js" - -export const Plugin = define({ - id: "opencode.config.compatibility", - effect: Effect.fn(function* (ctx) { - const config = yield* Config.Service - const fs = yield* FSUtil.Service - const watcher = yield* Watcher.Service - const watches = yield* FiberMap.make() - const changes = yield* PubSub.sliding(1) - const lock = Semaphore.makeUnsafe(1) - const skills: Skill.Info[] = [] - - const watch = Effect.fn("ConfigCompatibilityPlugin.watch")(function* ( - target: string, - type: "file" | "directory", - ) { - const updates = yield* watcher.subscribe({ path: target, type }) - yield* FiberMap.run( - watches, - `${type}:${target}`, - updates.pipe(Stream.runForEach((update) => PubSub.publish(changes, update.path).pipe(Effect.asVoid))), - { onlyIfMissing: true, startImmediately: true }, - ) - }) - - const refresh = Effect.fn("ConfigCompatibilityPlugin.refresh")( - function* () { - yield* FiberMap.clear(watches) - const roots = config.compatibility ? yield* config.compatibility() : { claude: [], agents: [] } - const directories = [...roots.claude, ...roots.agents].map((root) => path.join(root, "skills")) - const loaded = new Map() - for (const directory of directories) { - const resolved = yield* fs.realPath(directory).pipe(Effect.orElseSucceed(() => undefined)) - if (!resolved) continue - yield* watch(resolved, "directory") - const files = yield* fs - .scan("{*.md,**/SKILL.md}", { cwd: resolved, absolute: true, include: "file", symlink: true, dot: true }) - .pipe(Effect.orElseSucceed(() => [] as string[])) - for (const filepath of files.toSorted()) { - const content = yield* fs.readFileStringSafe(filepath).pipe(Effect.orElseSucceed(() => undefined)) - if (!content) continue - const parsed = SkillFile.parse(resolved, filepath, content) - if (parsed._tag === "Parsed") loaded.set(parsed.skill.id, parsed.skill) - } - } - skills.splice(0, skills.length, ...loaded.values()) - }, - (effect) => lock.withPermit(effect), - ) - - const reload = refresh().pipe(Effect.andThen(ctx.skill.reload())) - const updates = yield* PubSub.subscribe(changes) - yield* Stream.fromSubscription(updates).pipe( - Stream.debounce("100 millis"), - Stream.runForEach(() => reload), - Effect.forkScoped({ startImmediately: true }), - ) - yield* config.changes().pipe( - Stream.runForEach(() => reload), - Effect.forkScoped({ startImmediately: true }), - ) - yield* refresh() - yield* ctx.skill.transform((editor) => { - for (const skill of skills) editor.add(skill) - }) - }), -}) diff --git a/packages/core/src/config/plugin/skill.ts b/packages/core/src/config/plugin/skill.ts index 1338cab3400b..a02f51310cbd 100644 --- a/packages/core/src/config/plugin/skill.ts +++ b/packages/core/src/config/plugin/skill.ts @@ -25,8 +25,14 @@ export const Plugin = define({ const global = yield* Global.Service const location = yield* Location.Service const watcher = yield* Watcher.Service - const loaded: { entries: Entry[]; skills: Skill.Info[] } = { + const compatibilityRoots = config.compatibility ?? (() => Effect.succeed({ claude: [], agents: [] })) + const loaded: { + entries: Entry[] + compatibility: { readonly claude: readonly AbsolutePath[]; readonly agents: readonly AbsolutePath[] } + skills: Skill.Info[] + } = { entries: yield* config.entries(), + compatibility: yield* compatibilityRoots(), skills: [], } const watches = yield* FiberMap.make() @@ -82,6 +88,9 @@ export const Plugin = define({ } const directories = loaded.entries.flatMap((entry) => (entry.type === "directory" ? [entry.path] : [])) const items = loaded.entries.flatMap((entry) => (entry.type === "document" ? (entry.info.skills ?? []) : [])) + for (const directory of [...loaded.compatibility.claude, ...loaded.compatibility.agents]) { + add(Skill.DirectorySource.make({ type: "directory", path: AbsolutePath.make(path.join(directory, "skills")) })) + } for (const directory of directories) { add(Skill.DirectorySource.make({ type: "directory", path: AbsolutePath.make(path.join(directory, "skill")) })) add(Skill.DirectorySource.make({ type: "directory", path: AbsolutePath.make(path.join(directory, "skills")) })) @@ -181,8 +190,13 @@ export const Plugin = define({ yield* ctx.event.subscribe().pipe( Stream.filter((event) => event.type === "config.updated"), Stream.runForEach(() => - config.entries().pipe( - Effect.tap((entries) => Effect.sync(() => (loaded.entries = entries))), + Effect.all({ entries: config.entries(), compatibility: compatibilityRoots() }).pipe( + Effect.tap((next) => + Effect.sync(() => { + loaded.entries = next.entries + loaded.compatibility = next.compatibility + }), + ), Effect.andThen(refresh()), Effect.andThen(ctx.skill.reload()), ), diff --git a/packages/core/src/plugin/internal.ts b/packages/core/src/plugin/internal.ts index bdb0eb239888..1691a3c7bbe8 100644 --- a/packages/core/src/plugin/internal.ts +++ b/packages/core/src/plugin/internal.ts @@ -25,7 +25,6 @@ import { ConfigReferencePlugin } from "../config/plugin/reference.js" import { ConfigShellPlugin } from "../config/plugin/shell.js" import { ConfigSnapshotPlugin } from "../config/plugin/snapshot.js" import { ConfigSkillPlugin } from "../config/plugin/skill.js" -import { ConfigCompatibilityPlugin } from "../config/plugin/compatibility.js" import { ConfigToolOutputPlugin } from "../config/plugin/tool-output.js" import { ConfigWebSearchPlugin } from "../config/plugin/websearch.js" import { ConfigWorktreePlugin } from "../config/plugin/worktree.js" @@ -237,7 +236,6 @@ const post = [ ConfigShellPlugin.Plugin, ConfigSnapshotPlugin.Plugin, ConfigToolOutputPlugin.Plugin, - ConfigCompatibilityPlugin.Plugin, ConfigSkillPlugin.Plugin, ConfigProviderPlugin.Plugin, ConfigWebSearchPlugin.Plugin, diff --git a/packages/core/test/config/config.test.ts b/packages/core/test/config/config.test.ts index 09b1de4d4a1c..81d922a81972 100644 --- a/packages/core/test/config/config.test.ts +++ b/packages/core/test/config/config.test.ts @@ -350,6 +350,34 @@ describe("Config", () => { ), ) + it.live("publishes config updates when compatibility roots appear", () => + Effect.acquireDisposable(Effect.promise(() => tmpdir())).pipe( + Effect.flatMap((tmp) => + Effect.gen(function* () { + const global = path.join(tmp.path, "global") + const project = path.join(tmp.path, "project") + const root = path.join(project, ".claude") + yield* Effect.promise(() => Promise.all([fs.mkdir(global), fs.mkdir(project)])) + return yield* Effect.gen(function* () { + const config = yield* Config.Service + const bus = yield* Bus.Service + const watcher = yield* Watcher.Test + expect((yield* config.compatibility!()).claude).not.toContain(AbsolutePath.make(root)) + const changed = yield* bus + .subscribe(Event.Updated) + .pipe(Stream.take(1), Stream.runDrain, Effect.forkScoped({ startImmediately: true })) + + yield* Effect.promise(() => fs.mkdir(root)) + yield* watcher.emit({ type: "create", path: root }) + + yield* Fiber.join(changed).pipe(Effect.timeout("2 seconds")) + expect((yield* config.compatibility!()).claude).toContain(AbsolutePath.make(root)) + }).pipe(Effect.provide(testLayer(project, global, project, undefined, Watcher.testLayer))) + }), + ), + ), + ) + // Real watcher on purpose: the regression this pins (a deleted config file's // watch being torn down, making recreation invisible) only reproduces with // path-faithful event delivery. diff --git a/packages/core/test/config/skill.test.ts b/packages/core/test/config/skill.test.ts index d226842e9464..cccfcb2cb306 100644 --- a/packages/core/test/config/skill.test.ts +++ b/packages/core/test/config/skill.test.ts @@ -5,7 +5,6 @@ import { Deferred, Effect, Fiber, Layer, Schema, Stream } from "effect" import { Config } from "@opencode/core/config" import { Directory, Document, type Entry, Info } from "@opencode/schema/config" import { ConfigSkillPlugin } from "@opencode/core/config/plugin/skill" -import { ConfigCompatibilityPlugin } from "@opencode/core/config/plugin/compatibility" import { SkillFile } from "@opencode/core/config/plugin/skill-file" import { AppNodeBuilder } from "@opencode/core/effect/app-node-builder" import { Watcher } from "@opencode/core/filesystem/watcher" @@ -61,12 +60,7 @@ const startEntries = Effect.fnUntraced(function* ( reload: service.reload, }, }) - yield* ConfigCompatibilityPlugin.Plugin.effect(pluginHost).pipe( - Effect.provide(Config.testLayer(entries, compatibility)), - ) - yield* ConfigSkillPlugin.Plugin.effect( - pluginHost, - ).pipe( + yield* ConfigSkillPlugin.Plugin.effect(pluginHost).pipe( Effect.provide(Config.testLayer(entries, compatibility)), Effect.provideService(SkillDiscovery.Service, discovery), Effect.provideService(Global.Service, Global.Service.of({ ...Global.make(), home })), @@ -391,18 +385,22 @@ describe("ConfigSkillPlugin.Plugin", () => { ), ) - it.live("follows missing source directories as their parents appear", () => + it.live("follows missing compatibility skill directories as their parents appear", () => Effect.acquireDisposable(Effect.promise(() => tmpdir())).pipe( Effect.flatMap((tmp) => Effect.gen(function* () { - const source = path.join(tmp.path, "generated", "skills") - const skill = yield* start([source], tmp.path) + const root = path.join(tmp.path, "generated") + const source = path.join(root, "skills") + const skill = yield* startEntries([], tmp.path, tmp.path, emptyDiscovery, { + claude: [AbsolutePath.make(root)], + agents: [], + }) const watcher = yield* Watcher.Test expect(yield* skill.list()).toEqual([]) - expect(yield* watcher.subscriptions()).toEqual([{ path: path.join(tmp.path, "generated"), type: "file" }]) + expect(yield* watcher.subscriptions()).toEqual([{ path: root, type: "file" }]) - yield* Effect.promise(() => fs.mkdir(path.join(tmp.path, "generated"))) - yield* emitAndWait({ type: "create", path: path.join(tmp.path, "generated") }) + yield* Effect.promise(() => fs.mkdir(root)) + yield* emitAndWait({ type: "create", path: root }) yield* Effect.promise(async () => { await fs.mkdir(path.join(source, "deploy"), { recursive: true }) await write(source, "deploy", "Deploy")