From ea04b23745b34a9cab0c5d27053398db65e0dbf6 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 25 Mar 2026 20:55:43 -0400 Subject: [PATCH 1/2] skill: use Effect.cached for load deduplication (#19165) --- packages/opencode/src/mcp/auth.ts | 4 +- packages/opencode/src/mcp/index.ts | 4 +- packages/opencode/src/skill/index.ts | 116 +++++++++++---------------- 3 files changed, 50 insertions(+), 74 deletions(-) diff --git a/packages/opencode/src/mcp/auth.ts b/packages/opencode/src/mcp/auth.ts index 3c2b93f337..54f2ce4957 100644 --- a/packages/opencode/src/mcp/auth.ts +++ b/packages/opencode/src/mcp/auth.ts @@ -3,7 +3,7 @@ import z from "zod" import { Global } from "../global" import { Effect, Layer, ServiceMap } from "effect" import { AppFileSystem } from "@/filesystem" -import { makeRunPromise } from "@/effect/run-service" +import { makeRuntime } from "@/effect/run-service" export namespace McpAuth { export const Tokens = z.object({ @@ -143,7 +143,7 @@ export namespace McpAuth { const defaultLayer = layer.pipe(Layer.provide(AppFileSystem.defaultLayer)) - const runPromise = makeRunPromise(Service, defaultLayer) + const { runPromise } = makeRuntime(Service, defaultLayer) // Async facades for backward compat (used by McpOAuthProvider, CLI) diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 748f4abf02..d114550fcd 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -26,7 +26,7 @@ import { TuiEvent } from "@/cli/cmd/tui/event" import open from "open" import { Effect, Layer, Option, ServiceMap, Stream } from "effect" import { InstanceState } from "@/effect/instance-state" -import { makeRunPromise } from "@/effect/run-service" +import { makeRuntime } from "@/effect/run-service" import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process" import * as CrossSpawnSpawner from "@/effect/cross-spawn-spawner" import { NodeFileSystem } from "@effect/platform-node" @@ -893,7 +893,7 @@ export namespace MCP { Layer.provide(NodePath.layer), ) - const runPromise = makeRunPromise(Service, defaultLayer) + const { runPromise } = makeRuntime(Service, defaultLayer) // --- Async facade functions --- diff --git a/packages/opencode/src/skill/index.ts b/packages/opencode/src/skill/index.ts index 239549a1af..aa3829683a 100644 --- a/packages/opencode/src/skill/index.ts +++ b/packages/opencode/src/skill/index.ts @@ -54,11 +54,6 @@ export namespace Skill { type State = { skills: Record dirs: Set - task?: Promise - } - - type Cache = State & { - ensure: () => Promise } export interface Interface { @@ -116,66 +111,47 @@ export namespace Skill { }) } - // TODO: Migrate to Effect - const create = (discovery: Discovery.Interface, directory: string, worktree: string): Cache => { - const state: State = { - skills: {}, - dirs: new Set(), + async function loadSkills(state: State, discovery: Discovery.Interface, directory: string, worktree: string) { + if (!Flag.OPENCODE_DISABLE_EXTERNAL_SKILLS) { + for (const dir of EXTERNAL_DIRS) { + const root = path.join(Global.Path.home, dir) + if (!(await Filesystem.isDir(root))) continue + await scan(state, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "global" }) + } + + for await (const root of Filesystem.up({ + targets: EXTERNAL_DIRS, + start: directory, + stop: worktree, + })) { + await scan(state, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "project" }) + } } - const load = async () => { - if (!Flag.OPENCODE_DISABLE_EXTERNAL_SKILLS) { - for (const dir of EXTERNAL_DIRS) { - const root = path.join(Global.Path.home, dir) - if (!(await Filesystem.isDir(root))) continue - await scan(state, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "global" }) - } + for (const dir of await Config.directories()) { + await scan(state, dir, OPENCODE_SKILL_PATTERN) + } - for await (const root of Filesystem.up({ - targets: EXTERNAL_DIRS, - start: directory, - stop: worktree, - })) { - await scan(state, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "project" }) - } + const cfg = await Config.get() + for (const item of cfg.skills?.paths ?? []) { + const expanded = item.startsWith("~/") ? path.join(os.homedir(), item.slice(2)) : item + const dir = path.isAbsolute(expanded) ? expanded : path.join(directory, expanded) + if (!(await Filesystem.isDir(dir))) { + log.warn("skill path not found", { path: dir }) + continue } - for (const dir of await Config.directories()) { - await scan(state, dir, OPENCODE_SKILL_PATTERN) - } - - const cfg = await Config.get() - for (const item of cfg.skills?.paths ?? []) { - const expanded = item.startsWith("~/") ? path.join(os.homedir(), item.slice(2)) : item - const dir = path.isAbsolute(expanded) ? expanded : path.join(directory, expanded) - if (!(await Filesystem.isDir(dir))) { - log.warn("skill path not found", { path: dir }) - continue - } + await scan(state, dir, SKILL_PATTERN) + } + for (const url of cfg.skills?.urls ?? []) { + for (const dir of await Effect.runPromise(discovery.pull(url))) { + state.dirs.add(dir) await scan(state, dir, SKILL_PATTERN) } - - for (const url of cfg.skills?.urls ?? []) { - for (const dir of await Effect.runPromise(discovery.pull(url))) { - state.dirs.add(dir) - await scan(state, dir, SKILL_PATTERN) - } - } - - log.info("init", { count: Object.keys(state.skills).length }) } - const ensure = () => { - if (state.task) return state.task - state.task = load().catch((err) => { - state.task = undefined - throw err - }) - return state.task - } - - return { ...state, ensure } + log.info("init", { count: Object.keys(state.skills).length }) } export class Service extends ServiceMap.Service()("@opencode/Skill") {} @@ -185,33 +161,33 @@ export namespace Skill { Effect.gen(function* () { const discovery = yield* Discovery.Service const state = yield* InstanceState.make( - Effect.fn("Skill.state")((ctx) => Effect.sync(() => create(discovery, ctx.directory, ctx.worktree))), + Effect.fn("Skill.state")((ctx) => + Effect.gen(function* () { + const s: State = { skills: {}, dirs: new Set() } + yield* Effect.promise(() => loadSkills(s, discovery, ctx.directory, ctx.worktree)) + return s + }), + ), ) - const ensure = Effect.fn("Skill.ensure")(function* () { - const cache = yield* InstanceState.get(state) - yield* Effect.promise(() => cache.ensure()) - return cache - }) - const get = Effect.fn("Skill.get")(function* (name: string) { - const cache = yield* ensure() - return cache.skills[name] + const s = yield* InstanceState.get(state) + return s.skills[name] }) const all = Effect.fn("Skill.all")(function* () { - const cache = yield* ensure() - return Object.values(cache.skills) + const s = yield* InstanceState.get(state) + return Object.values(s.skills) }) const dirs = Effect.fn("Skill.dirs")(function* () { - const cache = yield* ensure() - return Array.from(cache.dirs) + const s = yield* InstanceState.get(state) + return Array.from(s.dirs) }) const available = Effect.fn("Skill.available")(function* (agent?: Agent.Info) { - const cache = yield* ensure() - const list = Object.values(cache.skills).toSorted((a, b) => a.name.localeCompare(b.name)) + const s = yield* InstanceState.get(state) + const list = Object.values(s.skills).toSorted((a, b) => a.name.localeCompare(b.name)) if (!agent) return list return list.filter((skill) => Permission.evaluate("skill", skill.name, agent.permission).action !== "deny") }) From 31ad6e85ba62ff6d7ad375a584f5ea8a3b66aef3 Mon Sep 17 00:00:00 2001 From: "opencode-agent[bot]" Date: Thu, 26 Mar 2026 00:56:34 +0000 Subject: [PATCH 2/2] chore: generate --- packages/opencode/specs/effect-migration.md | 37 ++++++++++++--------- packages/opencode/src/file/index.ts | 8 ++--- packages/opencode/src/plugin/index.ts | 22 ++++++------ packages/opencode/src/project/vcs.ts | 30 ++++++++--------- packages/opencode/test/project/vcs.test.ts | 5 +-- 5 files changed, 49 insertions(+), 53 deletions(-) diff --git a/packages/opencode/specs/effect-migration.md b/packages/opencode/specs/effect-migration.md index c95d131dc5..43b3194858 100644 --- a/packages/opencode/specs/effect-migration.md +++ b/packages/opencode/specs/effect-migration.md @@ -82,31 +82,38 @@ The `InstanceState.make` init callback receives a `Scope`, so you can use `Effec - **Subscriptions**: Yield `Bus.Service` at the layer level, then use `Stream` + `forkScoped` inside the init closure. The fiber is automatically interrupted when the instance scope closes: ```ts -const bus = yield* Bus.Service +const bus = yield * Bus.Service -const cache = yield* InstanceState.make( - Effect.fn("Foo.state")(function* (ctx) { - // ... load state ... +const cache = + yield * + InstanceState.make( + Effect.fn("Foo.state")(function* (ctx) { + // ... load state ... - yield* bus - .subscribeAll() - .pipe( - Stream.runForEach((event) => Effect.sync(() => { /* handle */ })), + yield* bus.subscribeAll().pipe( + Stream.runForEach((event) => + Effect.sync(() => { + /* handle */ + }), + ), Effect.forkScoped, ) - return { /* state */ } - }), -) + return { + /* state */ + } + }), + ) ``` - **Resource cleanup**: Use `Effect.acquireRelease` or `Effect.addFinalizer` for resources that need teardown (native watchers, process handles, etc.): ```ts -yield* Effect.acquireRelease( - Effect.sync(() => nativeAddon.watch(dir)), - (watcher) => Effect.sync(() => watcher.close()), -) +yield * + Effect.acquireRelease( + Effect.sync(() => nativeAddon.watch(dir)), + (watcher) => Effect.sync(() => watcher.close()), + ) ``` - **Background fibers**: Use `Effect.forkScoped` — the fiber is interrupted on disposal. diff --git a/packages/opencode/src/file/index.ts b/packages/opencode/src/file/index.ts index 61f1af89d3..efc3fa7c9a 100644 --- a/packages/opencode/src/file/index.ts +++ b/packages/opencode/src/file/index.ts @@ -404,15 +404,11 @@ export namespace File { s.cache = next }) - let cachedScan = yield* Effect.cached( - scan().pipe(Effect.catchCause(() => Effect.void)), - ) + let cachedScan = yield* Effect.cached(scan().pipe(Effect.catchCause(() => Effect.void))) const ensure = Effect.fn("File.ensure")(function* () { yield* cachedScan - cachedScan = yield* Effect.cached( - scan().pipe(Effect.catchCause(() => Effect.void)), - ) + cachedScan = yield* Effect.cached(scan().pipe(Effect.catchCause(() => Effect.void))) }) const init = Effect.fn("File.init")(function* () { diff --git a/packages/opencode/src/plugin/index.ts b/packages/opencode/src/plugin/index.ts index cd4b917992..9804169a46 100644 --- a/packages/opencode/src/plugin/index.ts +++ b/packages/opencode/src/plugin/index.ts @@ -149,18 +149,16 @@ export namespace Plugin { }) // Subscribe to bus events, fiber interrupted when scope closes - yield* bus - .subscribeAll() - .pipe( - Stream.runForEach((input) => - Effect.sync(() => { - for (const hook of hooks) { - hook["event"]?.({ event: input as any }) - } - }), - ), - Effect.forkScoped, - ) + yield* bus.subscribeAll().pipe( + Stream.runForEach((input) => + Effect.sync(() => { + for (const hook of hooks) { + hook["event"]?.({ event: input as any }) + } + }), + ), + Effect.forkScoped, + ) return { hooks } }), diff --git a/packages/opencode/src/project/vcs.ts b/packages/opencode/src/project/vcs.ts index 7df9dfb6f1..daca1c75c0 100644 --- a/packages/opencode/src/project/vcs.ts +++ b/packages/opencode/src/project/vcs.ts @@ -159,22 +159,20 @@ export namespace Vcs { const value = { current, root } log.info("initialized", { branch: value.current, default_branch: value.root?.name }) - yield* bus - .subscribe(FileWatcher.Event.Updated) - .pipe( - Stream.filter((evt) => evt.properties.file.endsWith("HEAD")), - Stream.runForEach((_evt) => - Effect.gen(function* () { - const next = yield* Effect.promise(() => get()) - if (next !== value.current) { - log.info("branch changed", { from: value.current, to: next }) - value.current = next - yield* bus.publish(Event.BranchUpdated, { branch: next }) - } - }), - ), - Effect.forkScoped, - ) + yield* bus.subscribe(FileWatcher.Event.Updated).pipe( + Stream.filter((evt) => evt.properties.file.endsWith("HEAD")), + Stream.runForEach((_evt) => + Effect.gen(function* () { + const next = yield* Effect.promise(() => get()) + if (next !== value.current) { + log.info("branch changed", { from: value.current, to: next }) + value.current = next + yield* bus.publish(Event.BranchUpdated, { branch: next }) + } + }), + ), + Effect.forkScoped, + ) return value }), diff --git a/packages/opencode/test/project/vcs.test.ts b/packages/opencode/test/project/vcs.test.ts index 50282b5f6c..a327f65fab 100644 --- a/packages/opencode/test/project/vcs.test.ts +++ b/packages/opencode/test/project/vcs.test.ts @@ -162,10 +162,7 @@ describe("Vcs diff", () => { await $`git worktree add -b feature/test ${dir} HEAD`.cwd(tmp.path).quiet() await withVcsOnly(dir, async () => { - const [branch, base] = await Promise.all([ - Vcs.branch(), - Vcs.defaultBranch(), - ]) + const [branch, base] = await Promise.all([Vcs.branch(), Vcs.defaultBranch()]) expect(branch).toBe("feature/test") expect(base).toBe("main") })