refactor(skill): effectify SkillService as scoped service (#17849)

Author: kitlangton

packages/opencode/package.json

@@ -95,6 +95,7 @@
     "@openrouter/ai-sdk-provider": "1.5.4",
     "@opentui/core": "0.1.87",
     "@opentui/solid": "0.1.87",
+    "@effect/platform-node": "4.0.0-beta.31",
     "@parcel/watcher": "2.5.1",
     "@pierre/diffs": "catalog:",
     "@solid-primitives/event-bus": "1.1.2",

packages/opencode/src/effect/instances.ts

@@ -9,6 +9,7 @@ import { VcsService } from "@/project/vcs"
 import { FileTimeService } from "@/file/time"
 import { FormatService } from "@/format"
 import { FileService } from "@/file"
+import { SkillService } from "@/skill/skill"
 import { Instance } from "@/project/instance"
 
 export { InstanceContext } from "./instance-context"
@@ -22,6 +23,7 @@ export type InstanceServices =
   | FileTimeService
   | FormatService
   | FileService
+  | SkillService
 
 function lookup(directory: string) {
   const project = Instance.project
@@ -35,6 +37,7 @@ function lookup(directory: string) {
     Layer.fresh(FileTimeService.layer).pipe(Layer.orDie),
     Layer.fresh(FormatService.layer),
     Layer.fresh(FileService.layer),
+    Layer.fresh(SkillService.layer),
   ).pipe(Layer.provide(ctx))
 }
 

packages/opencode/src/skill/discovery.ts

@@ -1,98 +1,118 @@
-import path from "path"
-import { mkdir } from "fs/promises"
-import { Log } from "../util/log"
+import { NodeFileSystem, NodePath } from "@effect/platform-node"
+import { Effect, FileSystem, Layer, Path, Schema, ServiceMap } from "effect"
+import { FetchHttpClient, HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http"
 import { Global } from "../global"
-import { Filesystem } from "../util/filesystem"
+import { Log } from "../util/log"
+import { withTransientReadRetry } from "@/util/effect-http-client"
 
-export namespace Discovery {
-  const log = Log.create({ service: "skill-discovery" })
+class IndexSkill extends Schema.Class<IndexSkill>("IndexSkill")({
+  name: Schema.String,
+  files: Schema.Array(Schema.String),
+}) {}
 
-  type Index = {
-    skills: Array<{
-      name: string
-      description: string
-      files: string[]
-    }>
-  }
+class Index extends Schema.Class<Index>("Index")({
+  skills: Schema.Array(IndexSkill),
+}) {}
 
-  export function dir() {
-    return path.join(Global.Path.cache, "skills")
-  }
+const skillConcurrency = 4
+const fileConcurrency = 8
 
-  async function get(url: string, dest: string): Promise<boolean> {
-    if (await Filesystem.exists(dest)) return true
-    return fetch(url)
-      .then(async (response) => {
-        if (!response.ok) {
-          log.error("failed to download", { url, status: response.status })
-          return false
-        }
-        if (response.body) await Filesystem.writeStream(dest, response.body)
-        return true
-      })
-      .catch((err) => {
-        log.error("failed to download", { url, err })
-        return false
-      })
+export namespace DiscoveryService {
+  export interface Service {
+    readonly pull: (url: string) => Effect.Effect<string[]>
   }
+}
 
-  export async function pull(url: string): Promise<string[]> {
-    const result: string[] = []
-    const base = url.endsWith("/") ? url : `${url}/`
-    const index = new URL("index.json", base).href
-    const cache = dir()
-    const host = base.slice(0, -1)
-
-    log.info("fetching index", { url: index })
-    const data = await fetch(index)
-      .then(async (response) => {
-        if (!response.ok) {
-          log.error("failed to fetch index", { url: index, status: response.status })
-          return undefined
-        }
-        return response
-          .json()
-          .then((json) => json as Index)
-          .catch((err) => {
-            log.error("failed to parse index", { url: index, err })
-            return undefined
-          })
-      })
-      .catch((err) => {
-        log.error("failed to fetch index", { url: index, err })
-        return undefined
+export class DiscoveryService extends ServiceMap.Service<DiscoveryService, DiscoveryService.Service>()(
+  "@opencode/SkillDiscovery",
+) {
+  static readonly layer = Layer.effect(
+    DiscoveryService,
+    Effect.gen(function* () {
+      const log = Log.create({ service: "skill-discovery" })
+      const fs = yield* FileSystem.FileSystem
+      const path = yield* Path.Path
+      const http = HttpClient.filterStatusOk(withTransientReadRetry(yield* HttpClient.HttpClient))
+      const cache = path.join(Global.Path.cache, "skills")
+
+      const download = Effect.fn("DiscoveryService.download")(function* (url: string, dest: string) {
+        if (yield* fs.exists(dest).pipe(Effect.orDie)) return true
+
+        return yield* HttpClientRequest.get(url).pipe(
+          http.execute,
+          Effect.flatMap((res) => res.arrayBuffer),
+          Effect.flatMap((body) =>
+            fs
+              .makeDirectory(path.dirname(dest), { recursive: true })
+              .pipe(Effect.flatMap(() => fs.writeFile(dest, new Uint8Array(body)))),
+          ),
+          Effect.as(true),
+          Effect.catch((err) =>
+            Effect.sync(() => {
+              log.error("failed to download", { url, err })
+              return false
+            }),
+          ),
+        )
       })
 
-    if (!data?.skills || !Array.isArray(data.skills)) {
-      log.warn("invalid index format", { url: index })
-      return result
-    }
-
-    const list = data.skills.filter((skill) => {
-      if (!skill?.name || !Array.isArray(skill.files)) {
-        log.warn("invalid skill entry", { url: index, skill })
-        return false
-      }
-      return true
-    })
-
-    await Promise.all(
-      list.map(async (skill) => {
-        const root = path.join(cache, skill.name)
-        await Promise.all(
-          skill.files.map(async (file) => {
-            const link = new URL(file, `${host}/${skill.name}/`).href
-            const dest = path.join(root, file)
-            await mkdir(path.dirname(dest), { recursive: true })
-            await get(link, dest)
-          }),
+      const pull: DiscoveryService.Service["pull"] = Effect.fn("DiscoveryService.pull")(function* (url: string) {
+        const base = url.endsWith("/") ? url : `${url}/`
+        const index = new URL("index.json", base).href
+        const host = base.slice(0, -1)
+
+        log.info("fetching index", { url: index })
+
+        const data = yield* HttpClientRequest.get(index).pipe(
+          HttpClientRequest.acceptJson,
+          http.execute,
+          Effect.flatMap(HttpClientResponse.schemaBodyJson(Index)),
+          Effect.catch((err) =>
+            Effect.sync(() => {
+              log.error("failed to fetch index", { url: index, err })
+              return null
+            }),
+          ),
         )
 
-        const md = path.join(root, "SKILL.md")
-        if (await Filesystem.exists(md)) result.push(root)
-      }),
-    )
+        if (!data) return []
 
-    return result
-  }
+        const list = data.skills.filter((skill) => {
+          if (!skill.files.includes("SKILL.md")) {
+            log.warn("skill entry missing SKILL.md", { url: index, skill: skill.name })
+            return false
+          }
+          return true
+        })
+
+        const dirs = yield* Effect.forEach(
+          list,
+          (skill) =>
+            Effect.gen(function* () {
+              const root = path.join(cache, skill.name)
+
+              yield* Effect.forEach(
+                skill.files,
+                (file) => download(new URL(file, `${host}/${skill.name}/`).href, path.join(root, file)),
+                { concurrency: fileConcurrency },
+              )
+
+              const md = path.join(root, "SKILL.md")
+              return (yield* fs.exists(md).pipe(Effect.orDie)) ? root : null
+            }),
+          { concurrency: skillConcurrency },
+        )
+
+        return dirs.filter((dir): dir is string => dir !== null)
+      })
+
+      return DiscoveryService.of({ pull })
+    }),
+  )
+
+  static readonly defaultLayer = DiscoveryService.layer.pipe(
+    Layer.provide(FetchHttpClient.layer),
+    Layer.provide(NodeFileSystem.layer),
+    Layer.provide(NodePath.layer),
+  )
 }