Просмотр исходного кода

fix(skill): type expected skill failures (#28885)

Shoubhit Dash 2 месяцев назад
Родитель
Сommit
7265c46af6

+ 22 - 5
packages/opencode/src/skill/index.ts

@@ -57,17 +57,26 @@ function isSkillFrontmatter(data: unknown): data is { name: string; description?
   )
   )
 }
 }
 
 
-export const InvalidError = NamedError.create("SkillInvalidError", {
+export class InvalidError extends Schema.TaggedErrorClass<InvalidError>()("SkillInvalidError", {
   path: Schema.String,
   path: Schema.String,
   message: Schema.optional(Schema.String),
   message: Schema.optional(Schema.String),
   issues: Schema.optional(Schema.Array(Issue)),
   issues: Schema.optional(Schema.Array(Issue)),
-})
+}) {}
 
 
-export const NameMismatchError = NamedError.create("SkillNameMismatchError", {
+export class NameMismatchError extends Schema.TaggedErrorClass<NameMismatchError>()("SkillNameMismatchError", {
   path: Schema.String,
   path: Schema.String,
   expected: Schema.String,
   expected: Schema.String,
   actual: Schema.String,
   actual: Schema.String,
-})
+}) {}
+
+export class NotFoundError extends Schema.TaggedErrorClass<NotFoundError>()("Skill.NotFoundError", {
+  name: Schema.String,
+  available: Schema.Array(Schema.String),
+}) {
+  override get message() {
+    return `Skill "${this.name}" not found. Available skills: ${this.available.join(", ") || "none"}`
+  }
+}
 
 
 type State = {
 type State = {
   skills: Record<string, Info>
   skills: Record<string, Info>
@@ -86,6 +95,7 @@ type ScanState = {
 
 
 export interface Interface {
 export interface Interface {
   readonly get: (name: string) => Effect.Effect<Info | undefined>
   readonly get: (name: string) => Effect.Effect<Info | undefined>
+  readonly require: (name: string) => Effect.Effect<Info, NotFoundError>
   readonly all: () => Effect.Effect<Info[]>
   readonly all: () => Effect.Effect<Info[]>
   readonly dirs: () => Effect.Effect<string[]>
   readonly dirs: () => Effect.Effect<string[]>
   readonly available: (agent?: Agent.Info) => Effect.Effect<Info[]>
   readonly available: (agent?: Agent.Info) => Effect.Effect<Info[]>
@@ -277,6 +287,13 @@ export const layer = Layer.effect(
       return s.skills[name]
       return s.skills[name]
     })
     })
 
 
+    const require = Effect.fn("Skill.require")(function* (name: string) {
+      const s = yield* InstanceState.get(state)
+      const info = s.skills[name]
+      if (info) return info
+      return yield* new NotFoundError({ name, available: Object.keys(s.skills).toSorted() })
+    })
+
     const all = Effect.fn("Skill.all")(function* () {
     const all = Effect.fn("Skill.all")(function* () {
       const s = yield* InstanceState.get(state)
       const s = yield* InstanceState.get(state)
       return Object.values(s.skills)
       return Object.values(s.skills)
@@ -293,7 +310,7 @@ export const layer = Layer.effect(
       return list.filter((skill) => Permission.evaluate("skill", skill.name, agent.permission).action !== "deny")
       return list.filter((skill) => Permission.evaluate("skill", skill.name, agent.permission).action !== "deny")
     })
     })
 
 
-    return Service.of({ get, all, dirs, available })
+    return Service.of({ get, require, all, dirs, available })
   }),
   }),
 )
 )
 
 

+ 3 - 6
packages/opencode/src/tool/skill.ts

@@ -22,12 +22,9 @@ export const SkillTool = Tool.define(
       parameters: Parameters,
       parameters: Parameters,
       execute: (params: Schema.Schema.Type<typeof Parameters>, ctx: Tool.Context) =>
       execute: (params: Schema.Schema.Type<typeof Parameters>, ctx: Tool.Context) =>
         Effect.gen(function* () {
         Effect.gen(function* () {
-          const info = yield* skill.get(params.name)
-          if (!info) {
-            const all = yield* skill.all()
-            const available = all.map((item) => item.name).join(", ")
-            throw new Error(`Skill "${params.name}" not found. Available skills: ${available || "none"}`)
-          }
+          const info = yield* skill
+            .require(params.name)
+            .pipe(Effect.catchTag("Skill.NotFoundError", (error) => Effect.die(new Error(error.message))))
 
 
           yield* ctx.ask({
           yield* ctx.ask({
             permission: "skill",
             permission: "skill",

+ 5 - 0
packages/opencode/test/session/system.test.ts

@@ -47,6 +47,11 @@ const it = testEffect(
         Skill.Service,
         Skill.Service,
         Skill.Service.of({
         Skill.Service.of({
           get: (name) => Effect.succeed(skills.find((skill) => skill.name === name)),
           get: (name) => Effect.succeed(skills.find((skill) => skill.name === name)),
+          require: (name) => {
+            const info = skills.find((skill) => skill.name === name)
+            if (info) return Effect.succeed(info)
+            return Effect.fail(new Skill.NotFoundError({ name, available: skills.map((skill) => skill.name) }))
+          },
           all: () => Effect.succeed(skills),
           all: () => Effect.succeed(skills),
           dirs: () => Effect.succeed([]),
           dirs: () => Effect.succeed([]),
           available: () => Effect.succeed(skills),
           available: () => Effect.succeed(skills),

+ 31 - 0
packages/opencode/test/skill/skill.test.ts

@@ -289,6 +289,37 @@ description: A skill in the .claude/skills directory.
     ),
     ),
   )
   )
 
 
+  it.live("fails with typed error when requiring a missing skill", () =>
+    provideTmpdirInstance(
+      () =>
+        Effect.gen(function* () {
+          const skill = yield* Skill.Service
+          const error = yield* Effect.flip(skill.require("missing-skill"))
+          expect(error).toBeInstanceOf(Skill.NotFoundError)
+          expect(error._tag).toBe("Skill.NotFoundError")
+          expect(error.name).toBe("missing-skill")
+          expect(error.message).toContain('Skill "missing-skill" not found.')
+        }),
+      { git: true },
+    ),
+  )
+
+  it.effect("exposes tagged expected skill failure classes", () =>
+    Effect.sync(() => {
+      const invalid = new Skill.InvalidError({ path: "/tmp/SKILL.md", message: "Invalid skill frontmatter" })
+      const mismatch = new Skill.NameMismatchError({
+        path: "/tmp/SKILL.md",
+        expected: "expected-skill",
+        actual: "actual-skill",
+      })
+
+      expect(invalid).toBeInstanceOf(Skill.InvalidError)
+      expect(invalid._tag).toBe("SkillInvalidError")
+      expect(mismatch).toBeInstanceOf(Skill.NameMismatchError)
+      expect(mismatch._tag).toBe("SkillNameMismatchError")
+    }),
+  )
+
   it.live("discovers skills from .agents/skills/ directory", () =>
   it.live("discovers skills from .agents/skills/ directory", () =>
     provideTmpdirInstance(
     provideTmpdirInstance(
       (dir) =>
       (dir) =>

+ 41 - 1
packages/opencode/test/tool/skill.test.ts

@@ -1,5 +1,5 @@
 import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
 import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
-import { Effect, Layer } from "effect"
+import { Cause, Effect, Exit, Layer } from "effect"
 import { afterEach, describe, expect } from "bun:test"
 import { afterEach, describe, expect } from "bun:test"
 import path from "path"
 import path from "path"
 import { pathToFileURL } from "url"
 import { pathToFileURL } from "url"
@@ -90,4 +90,44 @@ Use this skill.
       }),
       }),
     ),
     ),
   )
   )
+
+  it.live("execute preserves not found message", () =>
+    provideTmpdirInstance((dir) =>
+      Effect.gen(function* () {
+        const home = process.env.OPENCODE_TEST_HOME
+        process.env.OPENCODE_TEST_HOME = dir
+        yield* Effect.addFinalizer(() =>
+          Effect.sync(() => {
+            process.env.OPENCODE_TEST_HOME = home
+          }),
+        )
+
+        const registry = yield* ToolRegistry.Service
+        const agent = { name: "build", mode: "primary" as const, permission: [], options: {} }
+        const tool = (yield* registry.tools({
+          providerID: "opencode" as any,
+          modelID: "gpt-5" as any,
+          agent,
+        })).find((tool) => tool.id === SkillTool.id)
+        if (!tool) throw new Error("Skill tool not found")
+
+        const exit = yield* tool
+          .execute(
+            { name: "missing-skill" },
+            {
+              ...baseCtx,
+              ask: () => Effect.void,
+            },
+          )
+          .pipe(Effect.exit)
+
+        expect(Exit.isFailure(exit)).toBe(true)
+        if (Exit.isFailure(exit)) {
+          const error = Cause.squash(exit.cause)
+          expect(error).toBeInstanceOf(Error)
+          if (error instanceof Error) expect(error.message).toContain('Skill "missing-skill" not found.')
+        }
+      }),
+    ),
+  )
 })
 })