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

feat: add native skill tool with permission system (#5930)

Co-authored-by: Dax Raad <[email protected]>
Mohammad Alhashemi 3 месяцев назад
Родитель
Сommit
046e351140

+ 6 - 0
.opencode/skill/test-skill/SKILL.md

@@ -0,0 +1,6 @@
+---
+name: test-skill
+description: use this when asked to test skill
+---
+
+woah this is a test skill

+ 32 - 0
packages/opencode/src/agent/agent.ts

@@ -30,6 +30,7 @@ export namespace Agent {
       permission: z.object({
         edit: Config.Permission,
         bash: z.record(z.string(), Config.Permission),
+        skill: z.record(z.string(), Config.Permission),
         webfetch: Config.Permission.optional(),
         doom_loop: Config.Permission.optional(),
         external_directory: Config.Permission.optional(),
@@ -58,6 +59,9 @@ export namespace Agent {
       bash: {
         "*": "allow",
       },
+      skill: {
+        "*": "allow",
+      },
       webfetch: "allow",
       doom_loop: "ask",
       external_directory: "ask",
@@ -337,6 +341,17 @@ function mergeAgentPermissions(basePermission: any, overridePermission: any): Ag
       "*": overridePermission.bash,
     }
   }
+
+  if (typeof basePermission.skill === "string") {
+    basePermission.skill = {
+      "*": basePermission.skill,
+    }
+  }
+  if (typeof overridePermission.skill === "string") {
+    overridePermission.skill = {
+      "*": overridePermission.skill,
+    }
+  }
   const merged = mergeDeep(basePermission ?? {}, overridePermission ?? {}) as any
   let mergedBash
   if (merged.bash) {
@@ -354,10 +369,27 @@ function mergeAgentPermissions(basePermission: any, overridePermission: any): Ag
     }
   }
 
+  let mergedSkill
+  if (merged.skill) {
+    if (typeof merged.skill === "string") {
+      mergedSkill = {
+        "*": merged.skill,
+      }
+    } else if (typeof merged.skill === "object") {
+      mergedSkill = mergeDeep(
+        {
+          "*": "allow",
+        },
+        merged.skill,
+      )
+    }
+  }
+
   const result: Agent.Info["permission"] = {
     edit: merged.edit ?? "allow",
     webfetch: merged.webfetch ?? "allow",
     bash: mergedBash ?? { "*": "allow" },
+    skill: mergedSkill ?? { "*": "allow" },
     doom_loop: merged.doom_loop,
     external_directory: merged.external_directory,
   }

+ 2 - 0
packages/opencode/src/cli/cmd/debug/index.ts

@@ -6,6 +6,7 @@ import { FileCommand } from "./file"
 import { LSPCommand } from "./lsp"
 import { RipgrepCommand } from "./ripgrep"
 import { ScrapCommand } from "./scrap"
+import { SkillCommand } from "./skill"
 import { SnapshotCommand } from "./snapshot"
 
 export const DebugCommand = cmd({
@@ -17,6 +18,7 @@ export const DebugCommand = cmd({
       .command(RipgrepCommand)
       .command(FileCommand)
       .command(ScrapCommand)
+      .command(SkillCommand)
       .command(SnapshotCommand)
       .command(PathsCommand)
       .command({

+ 15 - 0
packages/opencode/src/cli/cmd/debug/skill.ts

@@ -0,0 +1,15 @@
+import { EOL } from "os"
+import { Skill } from "../../../skill"
+import { bootstrap } from "../../bootstrap"
+import { cmd } from "../cmd"
+
+export const SkillCommand = cmd({
+  command: "skill",
+  builder: (yargs) => yargs,
+  async handler() {
+    await bootstrap(process.cwd(), async () => {
+      const skills = await Skill.all()
+      process.stdout.write(JSON.stringify(skills, null, 2) + EOL)
+    })
+  },
+})

+ 2 - 0
packages/opencode/src/config/config.ts

@@ -414,6 +414,7 @@ export namespace Config {
         .object({
           edit: Permission.optional(),
           bash: z.union([Permission, z.record(z.string(), Permission)]).optional(),
+          skill: z.union([Permission, z.record(z.string(), Permission)]).optional(),
           webfetch: Permission.optional(),
           doom_loop: Permission.optional(),
           external_directory: Permission.optional(),
@@ -764,6 +765,7 @@ export namespace Config {
         .object({
           edit: Permission.optional(),
           bash: z.union([Permission, z.record(z.string(), Permission)]).optional(),
+          skill: z.union([Permission, z.record(z.string(), Permission)]).optional(),
           webfetch: Permission.optional(),
           doom_loop: Permission.optional(),
           external_directory: Permission.optional(),

+ 4 - 0
packages/opencode/src/session/compaction.ts

@@ -40,6 +40,8 @@ export namespace SessionCompaction {
   export const PRUNE_MINIMUM = 20_000
   export const PRUNE_PROTECT = 40_000
 
+  const PRUNE_PROTECTED_TOOLS = ["skill"]
+
   // goes backwards through parts until there are 40_000 tokens worth of tool
   // calls. then erases output of previous tool calls. idea is to throw away old
   // tool calls that are no longer relevant.
@@ -61,6 +63,8 @@ export namespace SessionCompaction {
         const part = msg.parts[partIndex]
         if (part.type === "tool")
           if (part.state.status === "completed") {
+            if (PRUNE_PROTECTED_TOOLS.includes(part.tool)) continue
+
             if (part.state.time.compacted) break loop
             const estimate = Token.estimate(part.state.output)
             total += estimate

+ 1 - 5
packages/opencode/src/session/prompt.ts

@@ -532,11 +532,7 @@ export namespace SessionPrompt {
         agent,
         abort,
         sessionID,
-        system: [
-          ...(await SystemPrompt.environment()),
-          ...(await SystemPrompt.skills()),
-          ...(await SystemPrompt.custom()),
-        ],
+        system: [...(await SystemPrompt.environment()), ...(await SystemPrompt.custom())],
         messages: [
           ...MessageV2.toModelMessage(sessionMessages),
           ...(isLastStep

+ 0 - 22
packages/opencode/src/session/system.ts

@@ -14,7 +14,6 @@ import PROMPT_POLARIS from "./prompt/polaris.txt"
 import PROMPT_BEAST from "./prompt/beast.txt"
 import PROMPT_GEMINI from "./prompt/gemini.txt"
 import PROMPT_ANTHROPIC_SPOOF from "./prompt/anthropic_spoof.txt"
-import PROMPT_COMPACTION from "./prompt/compaction.txt"
 
 import PROMPT_CODEX from "./prompt/codex.txt"
 import type { Provider } from "@/provider/provider"
@@ -118,25 +117,4 @@ export namespace SystemPrompt {
     )
     return Promise.all(found).then((result) => result.filter(Boolean))
   }
-
-  export async function skills() {
-    const all = await Skill.all()
-    if (all.length === 0) return []
-
-    const lines = [
-      "You have access to skills listed in `<available_skills>`. When a task matches a skill's description, read its SKILL.md file to get detailed instructions.",
-      "",
-      "<available_skills>",
-    ]
-    for (const skill of all) {
-      lines.push("  <skill>")
-      lines.push(`    <name>${skill.name}</name>`)
-      lines.push(`    <description>${skill.description}</description>`)
-      lines.push(`    <location>${skill.location}</location>`)
-      lines.push("  </skill>")
-    }
-    lines.push("</available_skills>")
-
-    return [lines.join("\n")]
-  }
 }

+ 26 - 109
packages/opencode/src/skill/skill.ts

@@ -1,43 +1,16 @@
-import path from "path"
 import z from "zod"
 import { Config } from "../config/config"
-import { Filesystem } from "../util/filesystem"
 import { Instance } from "../project/instance"
 import { NamedError } from "@opencode-ai/util/error"
 import { ConfigMarkdown } from "../config/markdown"
-import { Log } from "../util/log"
 
 export namespace Skill {
-  const log = Log.create({ service: "skill" })
-
-  // Name: 1-64 chars, lowercase alphanumeric and hyphens, no consecutive hyphens, can't start/end with hyphen
-  const NAME_REGEX = /^[a-z0-9]+(-[a-z0-9]+)*$/
-
-  export const Frontmatter = z.object({
-    name: z
-      .string()
-      .min(1)
-      .max(64)
-      .refine((val) => NAME_REGEX.test(val), {
-        message:
-          "Name must be lowercase alphanumeric with hyphens, no consecutive hyphens, cannot start or end with hyphen",
-      }),
-    description: z.string().min(1).max(1024),
-    license: z.string().optional(),
-    compatibility: z.string().max(500).optional(),
-    metadata: z.record(z.string(), z.string()).optional(),
+  export const Info = z.object({
+    name: z.string(),
+    description: z.string(),
+    location: z.string(),
   })
-
-  export type Frontmatter = z.infer<typeof Frontmatter>
-
-  export interface Info {
-    name: string
-    description: string
-    location: string
-    license?: string
-    compatibility?: string
-    metadata?: Record<string, string>
-  }
+  export type Info = z.infer<typeof Info>
 
   export const InvalidError = NamedError.create(
     "SkillInvalidError",
@@ -57,15 +30,12 @@ export namespace Skill {
     }),
   )
 
-  const SKILL_GLOB = new Bun.Glob("skill/*/SKILL.md")
-  // const CLAUDE_SKILL_GLOB = new Bun.Glob("*/SKILL.md")
+  const SKILL_GLOB = new Bun.Glob("skill/**/SKILL.md")
 
-  async function discover(): Promise<string[]> {
+  export const state = Instance.state(async () => {
     const directories = await Config.directories()
+    const skills: Record<string, Info> = {}
 
-    const paths: string[] = []
-
-    // Scan skill/ subdirectory in config directories (.opencode/, ~/.opencode/, etc.)
     for (const dir of directories) {
       for await (const match of SKILL_GLOB.scan({
         cwd: dir,
@@ -73,82 +43,29 @@ export namespace Skill {
         onlyFiles: true,
         followSymlinks: true,
       })) {
-        paths.push(match)
+        const md = await ConfigMarkdown.parse(match)
+        if (!md) {
+          continue
+        }
+
+        const parsed = Info.pick({ name: true, description: true }).safeParse(md.data)
+        if (!parsed.success) continue
+        skills[parsed.data.name] = {
+          name: parsed.data.name,
+          description: parsed.data.description,
+          location: match,
+        }
       }
     }
 
-    // Also scan .claude/skills/ walking up from cwd to worktree
-    // for await (const dir of Filesystem.up({
-    //   targets: [".claude/skills"],
-    //   start: Instance.directory,
-    //   stop: Instance.worktree,
-    // })) {
-    //   for await (const match of CLAUDE_SKILL_GLOB.scan({
-    //     cwd: dir,
-    //     absolute: true,
-    //     onlyFiles: true,
-    //     followSymlinks: true,
-    //   })) {
-    //     paths.push(match)
-    //   }
-    // }
-
-    return paths
-  }
-
-  async function load(skillMdPath: string): Promise<Info> {
-    const md = await ConfigMarkdown.parse(skillMdPath)
-    if (!md.data) {
-      throw new InvalidError({
-        path: skillMdPath,
-        message: "SKILL.md must have YAML frontmatter",
-      })
-    }
-
-    const parsed = Frontmatter.safeParse(md.data)
-    if (!parsed.success) {
-      throw new InvalidError({
-        path: skillMdPath,
-        issues: parsed.error.issues,
-      })
-    }
-
-    const frontmatter = parsed.data
-    const skillDir = path.dirname(skillMdPath)
-    const dirName = path.basename(skillDir)
-
-    if (frontmatter.name !== dirName) {
-      throw new NameMismatchError({
-        path: skillMdPath,
-        expected: dirName,
-        actual: frontmatter.name,
-      })
-    }
-
-    return {
-      name: frontmatter.name,
-      description: frontmatter.description,
-      location: skillMdPath,
-      license: frontmatter.license,
-      compatibility: frontmatter.compatibility,
-      metadata: frontmatter.metadata,
-    }
-  }
-
-  export const state = Instance.state(async () => {
-    const paths = await discover()
-    const skills: Info[] = []
-
-    for (const skillPath of paths) {
-      const info = await load(skillPath)
-      log.info("loaded skill", { name: info.name, location: info.location })
-      skills.push(info)
-    }
-
     return skills
   })
 
-  export async function all(): Promise<Info[]> {
-    return state()
+  export async function get(name: string) {
+    return state().then((x) => x[name])
+  }
+
+  export async function all() {
+    return state().then((x) => Object.values(x))
   }
 }

+ 6 - 0
packages/opencode/src/tool/registry.ts

@@ -10,6 +10,7 @@ import { TodoWriteTool, TodoReadTool } from "./todo"
 import { WebFetchTool } from "./webfetch"
 import { WriteTool } from "./write"
 import { InvalidTool } from "./invalid"
+import { SkillTool } from "./skill"
 import type { Agent } from "../agent/agent"
 import { Tool } from "./tool"
 import { Instance } from "../project/instance"
@@ -103,6 +104,7 @@ export namespace ToolRegistry {
       TodoReadTool,
       WebSearchTool,
       CodeSearchTool,
+      SkillTool,
       ...(Flag.OPENCODE_EXPERIMENTAL_LSP_TOOL ? [LspTool] : []),
       ...(config.experimental?.batch_tool === true ? [BatchTool] : []),
       ...custom,
@@ -150,6 +152,10 @@ export namespace ToolRegistry {
       result["codesearch"] = false
       result["websearch"] = false
     }
+    // Disable skill tool if all skills are denied
+    if (agent.permission.skill["*"] === "deny" && Object.keys(agent.permission.skill).length === 1) {
+      result["skill"] = false
+    }
 
     return result
   }

+ 84 - 0
packages/opencode/src/tool/skill.ts

@@ -0,0 +1,84 @@
+import path from "path"
+import z from "zod"
+import { Tool } from "./tool"
+import { Skill } from "../skill"
+import { Agent } from "../agent/agent"
+import { Permission } from "../permission"
+import { Wildcard } from "../util/wildcard"
+import { ConfigMarkdown } from "../config/markdown"
+
+export const SkillTool = Tool.define("skill", async () => {
+  const skills = await Skill.all()
+  return {
+    description: [
+      "Load a skill to get detailed instructions for a specific task.",
+      "Skills provide specialized knowledge and step-by-step guidance.",
+      "Use this when a task matches an available skill's description.",
+      "<available_skills>",
+      ...skills.flatMap((skill) => [
+        `  <skill>`,
+        `    <name>${skill.name}</name>`,
+        `    <description>${skill.description}</description>`,
+        `  </skill>`,
+      ]),
+      "</available_skills>",
+    ].join(" "),
+    parameters: z.object({
+      name: z
+        .string()
+        .describe("The skill identifier from available_skills (e.g., 'code-review' or 'category/helper')"),
+    }),
+    async execute(params, ctx) {
+      const agent = await Agent.get(ctx.agent)
+
+      const skill = await Skill.get(params.name)
+
+      if (!skill) {
+        const available = Skill.all().then((x) => Object.keys(x).join(", "))
+        throw new Error(`Skill "${params.name}" not found. Available skills: ${available || "none"}`)
+      }
+
+      // Check permission using Wildcard.all on the skill ID
+      const permissions = agent.permission.skill
+      const action = Wildcard.all(params.name, permissions)
+
+      if (action === "deny") {
+        throw new Permission.RejectedError(
+          ctx.sessionID,
+          "skill",
+          ctx.callID,
+          { skill: params.name },
+          `Access to skill "${params.name}" is denied for agent "${agent.name}".`,
+        )
+      }
+
+      if (action === "ask") {
+        await Permission.ask({
+          type: "skill",
+          pattern: params.name,
+          sessionID: ctx.sessionID,
+          messageID: ctx.messageID,
+          callID: ctx.callID,
+          title: `Load skill: ${skill.name}`,
+          metadata: { id: params.name, name: skill.name, description: skill.description },
+        })
+      }
+
+      // Load and parse skill content
+      const parsed = await ConfigMarkdown.parse(skill.location)
+      const dir = path.dirname(skill.location)
+
+      // Format output similar to plugin pattern
+      const output = [`## Skill: ${skill.name}`, "", `**Base directory**: ${dir}`, "", parsed.content.trim()].join("\n")
+
+      return {
+        title: `Loaded skill: ${skill.name}`,
+        output,
+        metadata: {
+          name: skill.name,
+          dir,
+        },
+      }
+    },
+  }
+})

+ 2 - 162
packages/opencode/test/skill/skill.test.ts

@@ -65,55 +65,7 @@ description: Another test skill.
   })
 })
 
-test("throws error for invalid skill name format", async () => {
-  await using tmp = await tmpdir({
-    git: true,
-    init: async (dir) => {
-      const skillDir = path.join(dir, ".opencode", "skill", "InvalidName")
-      await Bun.write(
-        path.join(skillDir, "SKILL.md"),
-        `---
-name: InvalidName
-description: A skill with invalid name.
----
-`,
-      )
-    },
-  })
-
-  await Instance.provide({
-    directory: tmp.path,
-    fn: async () => {
-      await expect(Skill.all()).rejects.toThrow()
-    },
-  })
-})
-
-test("throws error when name doesn't match directory", async () => {
-  await using tmp = await tmpdir({
-    git: true,
-    init: async (dir) => {
-      const skillDir = path.join(dir, ".opencode", "skill", "dir-name")
-      await Bun.write(
-        path.join(skillDir, "SKILL.md"),
-        `---
-name: different-name
-description: A skill with mismatched name.
----
-`,
-      )
-    },
-  })
-
-  await Instance.provide({
-    directory: tmp.path,
-    fn: async () => {
-      await expect(Skill.all()).rejects.toThrow("SkillNameMismatchError")
-    },
-  })
-})
-
-test("throws error for missing frontmatter", async () => {
+test("skips skills with missing frontmatter", async () => {
   await using tmp = await tmpdir({
     git: true,
     init: async (dir) => {
@@ -128,78 +80,11 @@ Just some content without YAML frontmatter.
     },
   })
 
-  await Instance.provide({
-    directory: tmp.path,
-    fn: async () => {
-      await expect(Skill.all()).rejects.toThrow("SkillInvalidError")
-    },
-  })
-})
-
-test("parses optional fields", async () => {
-  await using tmp = await tmpdir({
-    git: true,
-    init: async (dir) => {
-      const skillDir = path.join(dir, ".opencode", "skill", "full-skill")
-      await Bun.write(
-        path.join(skillDir, "SKILL.md"),
-        `---
-name: full-skill
-description: A skill with all optional fields.
-license: MIT
-compatibility: Requires Node.js 18+
-metadata:
-  author: test-author
-  version: "1.0"
----
-
-# Full Skill
-`,
-      )
-    },
-  })
-
-  await Instance.provide({
-    directory: tmp.path,
-    fn: async () => {
-      const skills = await Skill.all()
-      expect(skills.length).toBe(1)
-      expect(skills[0].license).toBe("MIT")
-      expect(skills[0].compatibility).toBe("Requires Node.js 18+")
-      expect(skills[0].metadata).toEqual({
-        author: "test-author",
-        version: "1.0",
-      })
-    },
-  })
-})
-
-test("ignores unknown frontmatter fields", async () => {
-  await using tmp = await tmpdir({
-    git: true,
-    init: async (dir) => {
-      const skillDir = path.join(dir, ".opencode", "skill", "extra-fields")
-      await Bun.write(
-        path.join(skillDir, "SKILL.md"),
-        `---
-name: extra-fields
-description: A skill with extra unknown fields.
-allowed-tools: Bash Read Write
-some-other-field: ignored
----
-
-# Extra Fields Skill
-`,
-      )
-    },
-  })
-
   await Instance.provide({
     directory: tmp.path,
     fn: async () => {
       const skills = await Skill.all()
-      expect(skills.length).toBe(1)
-      expect(skills[0].name).toBe("extra-fields")
+      expect(skills).toEqual([])
     },
   })
 })
@@ -216,51 +101,6 @@ test("returns empty array when no skills exist", async () => {
   })
 })
 
-test("SystemPrompt.skills() returns empty array when no skills", async () => {
-  await using tmp = await tmpdir({ git: true })
-
-  await Instance.provide({
-    directory: tmp.path,
-    fn: async () => {
-      const result = await SystemPrompt.skills()
-      expect(result).toEqual([])
-    },
-  })
-})
-
-test("SystemPrompt.skills() returns XML block with skills", async () => {
-  await using tmp = await tmpdir({
-    git: true,
-    init: async (dir) => {
-      const skillDir = path.join(dir, ".opencode", "skill", "example-skill")
-      await Bun.write(
-        path.join(skillDir, "SKILL.md"),
-        `---
-name: example-skill
-description: An example skill for testing XML output.
----
-
-# Example
-`,
-      )
-    },
-  })
-
-  await Instance.provide({
-    directory: tmp.path,
-    fn: async () => {
-      const result = await SystemPrompt.skills()
-      expect(result.length).toBe(1)
-      expect(result[0]).toContain("<available_skills>")
-      expect(result[0]).toContain("<name>example-skill</name>")
-      expect(result[0]).toContain("<description>An example skill for testing XML output.</description>")
-      expect(result[0]).toContain("SKILL.md</location>")
-      expect(result[0]).toContain("</available_skills>")
-      expect(result[0]).toContain("When a task matches a skill's description")
-    },
-  })
-})
-
 // test("discovers skills from .claude/skills/ directory", async () => {
 //   await using tmp = await tmpdir({
 //     git: true,