Browse Source

skill: use Effect.cached for load deduplication (#19165)

Kit Langton 3 weeks ago
parent
commit
ea04b23745

+ 2 - 2
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)
 

+ 2 - 2
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 ---
 

+ 45 - 69
packages/opencode/src/skill/index.ts

@@ -54,11 +54,6 @@ export namespace Skill {
   type State = {
     skills: Record<string, Info>
     dirs: Set<string>
-    task?: Promise<void>
-  }
-
-  type Cache = State & {
-    ensure: () => Promise<void>
   }
 
   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<string>(),
-    }
-
-    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 await (const root of Filesystem.up({
-          targets: EXTERNAL_DIRS,
-          start: directory,
-          stop: worktree,
-        })) {
-          await scan(state, root, EXTERNAL_SKILL_PATTERN, { dot: true, scope: "project" })
-        }
+  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 (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
-        }
-
-        await scan(state, dir, SKILL_PATTERN)
-      }
+    for (const dir of await Config.directories()) {
+      await scan(state, dir, OPENCODE_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)
-        }
+    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
       }
 
-      log.info("init", { count: Object.keys(state.skills).length })
+      await scan(state, dir, SKILL_PATTERN)
     }
 
-    const ensure = () => {
-      if (state.task) return state.task
-      state.task = load().catch((err) => {
-        state.task = undefined
-        throw err
-      })
-      return state.task
+    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)
+      }
     }
 
-    return { ...state, ensure }
+    log.info("init", { count: Object.keys(state.skills).length })
   }
 
   export class Service extends ServiceMap.Service<Service, Interface>()("@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")
       })