diff --git a/packages/opencode/src/skill/skill.ts b/packages/opencode/src/skill/skill.ts index 6ae0e9fe887..ea39ccc1930 100644 --- a/packages/opencode/src/skill/skill.ts +++ b/packages/opencode/src/skill/skill.ts @@ -14,9 +14,25 @@ import { Session } from "@/session" export namespace Skill { const log = Log.create({ service: "skill" }) + + /** + * Regex for skill name validation. + * - Lowercase alphanumeric with single hyphen separators + * - Cannot start or end with `-` + * - Cannot contain consecutive `--` + */ + export const NAME_REGEX = /^[a-z0-9]+(-[a-z0-9]+)*$/ + export const Info = z.object({ - name: z.string(), - description: z.string(), + name: z + .string() + .min(1, "Skill name must be at least 1 character") + .max(64, "Skill name must be at most 64 characters") + .regex(NAME_REGEX, "Skill name must be lowercase alphanumeric with single hyphen separators (e.g., 'my-skill')"), + description: z + .string() + .min(1, "Skill description must be at least 1 character") + .max(1024, "Skill description must be at most 1024 characters"), location: z.string(), }) export type Info = z.infer @@ -58,7 +74,24 @@ export namespace Skill { if (!md) return const parsed = Info.pick({ name: true, description: true }).safeParse(md.data) - if (!parsed.success) return + if (!parsed.success) { + log.error("invalid skill frontmatter", { + skill: match, + issues: parsed.error.issues.map((i) => i.message), + }) + return + } + + // Validate that skill name matches the directory name + const dirName = path.basename(path.dirname(match)) + if (parsed.data.name !== dirName) { + log.error("skill name must match directory name", { + skill: match, + expected: dirName, + actual: parsed.data.name, + }) + return + } // Warn on duplicate skill names if (skills[parsed.data.name]) { diff --git a/packages/opencode/test/skill/skill.test.ts b/packages/opencode/test/skill/skill.test.ts index 72415c1411e..8ec6fad1e9c 100644 --- a/packages/opencode/test/skill/skill.test.ts +++ b/packages/opencode/test/skill/skill.test.ts @@ -183,3 +183,76 @@ test("returns empty array when no skills exist", async () => { }, }) }) + +test("validates skill name format", () => { + // Valid names + expect(Skill.NAME_REGEX.test("my-skill")).toBe(true) + expect(Skill.NAME_REGEX.test("skill")).toBe(true) + expect(Skill.NAME_REGEX.test("skill123")).toBe(true) + expect(Skill.NAME_REGEX.test("my-test-skill")).toBe(true) + expect(Skill.NAME_REGEX.test("a")).toBe(true) + expect(Skill.NAME_REGEX.test("a1b2c3")).toBe(true) + + // Invalid names + expect(Skill.NAME_REGEX.test("-skill")).toBe(false) // starts with hyphen + expect(Skill.NAME_REGEX.test("skill-")).toBe(false) // ends with hyphen + expect(Skill.NAME_REGEX.test("my--skill")).toBe(false) // consecutive hyphens + expect(Skill.NAME_REGEX.test("My-Skill")).toBe(false) // uppercase + expect(Skill.NAME_REGEX.test("my_skill")).toBe(false) // underscore + expect(Skill.NAME_REGEX.test("my skill")).toBe(false) // space + expect(Skill.NAME_REGEX.test("")).toBe(false) // empty +}) + +test("skips skills with invalid name format", async () => { + await using tmp = await tmpdir({ + git: true, + init: async (dir) => { + const skillDir = path.join(dir, ".opencode", "skill", "My_Invalid_Skill") + await Bun.write( + path.join(skillDir, "SKILL.md"), + `--- +name: My_Invalid_Skill +description: A skill with invalid name format. +--- + +# Invalid Skill +`, + ) + }, + }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const skills = await Skill.all() + expect(skills).toEqual([]) + }, + }) +}) + +test("skips skills where name doesn't match directory", async () => { + await using tmp = await tmpdir({ + git: true, + init: async (dir) => { + const skillDir = path.join(dir, ".opencode", "skill", "actual-dir-name") + await Bun.write( + path.join(skillDir, "SKILL.md"), + `--- +name: different-name +description: A skill with mismatched name. +--- + +# Mismatched Skill +`, + ) + }, + }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const skills = await Skill.all() + expect(skills).toEqual([]) + }, + }) +})