refactor: improve code quality and add i18n for skills feature

- Extract shared skill name validation to packages/types/src/skills.ts (DRY)
- Replace hardcoded modes with getAllModes() from @roo/modes
- Remove race condition setTimeout calls (backend already sends updated skills)
- Consolidate duplicate getAllSkills/getSkillsMetadata methods
- Remove redundant type assertion in SkillsSettings
- Remove stale 'Phase 4' comments
- Remove unnecessary ExtendedExtensionState interface workaround
- Add i18n support for all skills error messages (17 locales)
- Add 21 tests for shared validation function

Author: daniel-lxs

packages/types/src/__tests__/skills.test.ts

@@ -0,0 +1,144 @@
+import {
+	validateSkillName,
+	SkillNameValidationError,
+	SKILL_NAME_MIN_LENGTH,
+	SKILL_NAME_MAX_LENGTH,
+	SKILL_NAME_REGEX,
+} from "../skills.js"
+
+describe("validateSkillName", () => {
+	describe("valid names", () => {
+		it("accepts single lowercase word", () => {
+			expect(validateSkillName("myskill")).toEqual({ valid: true })
+		})
+
+		it("accepts lowercase letters and numbers", () => {
+			expect(validateSkillName("skill123")).toEqual({ valid: true })
+		})
+
+		it("accepts hyphenated words", () => {
+			expect(validateSkillName("my-skill")).toEqual({ valid: true })
+		})
+
+		it("accepts multiple hyphenated words", () => {
+			expect(validateSkillName("my-awesome-skill")).toEqual({ valid: true })
+		})
+
+		it("accepts single character", () => {
+			expect(validateSkillName("a")).toEqual({ valid: true })
+		})
+
+		it("accepts single digit", () => {
+			expect(validateSkillName("1")).toEqual({ valid: true })
+		})
+
+		it("accepts maximum length name (64 characters)", () => {
+			const maxLengthName = "a".repeat(SKILL_NAME_MAX_LENGTH)
+			expect(validateSkillName(maxLengthName)).toEqual({ valid: true })
+		})
+	})
+
+	describe("empty or missing names", () => {
+		it("rejects empty string", () => {
+			expect(validateSkillName("")).toEqual({
+				valid: false,
+				error: SkillNameValidationError.Empty,
+			})
+		})
+	})
+
+	describe("names that are too long", () => {
+		it("rejects names longer than 64 characters", () => {
+			const tooLongName = "a".repeat(SKILL_NAME_MAX_LENGTH + 1)
+			expect(validateSkillName(tooLongName)).toEqual({
+				valid: false,
+				error: SkillNameValidationError.TooLong,
+			})
+		})
+	})
+
+	describe("invalid format", () => {
+		it("rejects uppercase letters", () => {
+			expect(validateSkillName("MySkill")).toEqual({
+				valid: false,
+				error: SkillNameValidationError.InvalidFormat,
+			})
+		})
+
+		it("rejects leading hyphen", () => {
+			expect(validateSkillName("-myskill")).toEqual({
+				valid: false,
+				error: SkillNameValidationError.InvalidFormat,
+			})
+		})
+
+		it("rejects trailing hyphen", () => {
+			expect(validateSkillName("myskill-")).toEqual({
+				valid: false,
+				error: SkillNameValidationError.InvalidFormat,
+			})
+		})
+
+		it("rejects consecutive hyphens", () => {
+			expect(validateSkillName("my--skill")).toEqual({
+				valid: false,
+				error: SkillNameValidationError.InvalidFormat,
+			})
+		})
+
+		it("rejects spaces", () => {
+			expect(validateSkillName("my skill")).toEqual({
+				valid: false,
+				error: SkillNameValidationError.InvalidFormat,
+			})
+		})
+
+		it("rejects underscores", () => {
+			expect(validateSkillName("my_skill")).toEqual({
+				valid: false,
+				error: SkillNameValidationError.InvalidFormat,
+			})
+		})
+
+		it("rejects special characters", () => {
+			expect(validateSkillName("my@skill")).toEqual({
+				valid: false,
+				error: SkillNameValidationError.InvalidFormat,
+			})
+		})
+
+		it("rejects dots", () => {
+			expect(validateSkillName("my.skill")).toEqual({
+				valid: false,
+				error: SkillNameValidationError.InvalidFormat,
+			})
+		})
+	})
+})
+
+describe("SKILL_NAME_REGEX", () => {
+	it("matches valid names", () => {
+		expect(SKILL_NAME_REGEX.test("myskill")).toBe(true)
+		expect(SKILL_NAME_REGEX.test("my-skill")).toBe(true)
+		expect(SKILL_NAME_REGEX.test("skill123")).toBe(true)
+		expect(SKILL_NAME_REGEX.test("a1-b2-c3")).toBe(true)
+	})
+
+	it("does not match invalid names", () => {
+		expect(SKILL_NAME_REGEX.test("-start")).toBe(false)
+		expect(SKILL_NAME_REGEX.test("end-")).toBe(false)
+		expect(SKILL_NAME_REGEX.test("double--hyphen")).toBe(false)
+		expect(SKILL_NAME_REGEX.test("UPPER")).toBe(false)
+		expect(SKILL_NAME_REGEX.test("")).toBe(false)
+	})
+})
+
+describe("constants", () => {
+	it("has correct min length", () => {
+		expect(SKILL_NAME_MIN_LENGTH).toBe(1)
+	})
+
+	it("has correct max length", () => {
+		expect(SKILL_NAME_MAX_LENGTH).toBe(64)
+	})
+})

packages/types/src/skills.ts

@@ -9,3 +9,63 @@ export interface SkillMetadata {
 	source: "global" | "project" // Where the skill was discovered
 	mode?: string // If set, skill is only available in this mode
 }
+
+/**
+ * Skill name validation constants per agentskills.io specification:
+ * https://agentskills.io/specification
+ *
+ * Name constraints:
+ * - 1-64 characters
+ * - Lowercase letters, numbers, and hyphens only
+ * - Must not start or end with a hyphen
+ * - Must not contain consecutive hyphens
+ */
+export const SKILL_NAME_MIN_LENGTH = 1
+export const SKILL_NAME_MAX_LENGTH = 64
+
+/**
+ * Regex pattern for valid skill names.
+ * Matches: lowercase letters/numbers, optionally followed by groups of hyphen + lowercase letters/numbers.
+ * This ensures no leading/trailing hyphens and no consecutive hyphens.
+ */
+export const SKILL_NAME_REGEX = /^[a-z0-9]+(?:-[a-z0-9]+)*$/
+
+/**
+ * Error codes for skill name validation.
+ * These can be mapped to translation keys in the frontend or error messages in the backend.
+ */
+export enum SkillNameValidationError {
+	Empty = "empty",
+	TooLong = "too_long",
+	InvalidFormat = "invalid_format",
+}
+
+/**
+ * Result of skill name validation.
+ */
+export interface SkillNameValidationResult {
+	valid: boolean
+	error?: SkillNameValidationError
+}
+
+/**
+ * Validate a skill name according to agentskills.io specification.
+ *
+ * @param name - The skill name to validate
+ * @returns Validation result with error code if invalid
+ */
+export function validateSkillName(name: string): SkillNameValidationResult {
+	if (!name || name.length < SKILL_NAME_MIN_LENGTH) {
+		return { valid: false, error: SkillNameValidationError.Empty }
+	}
+
+	if (name.length > SKILL_NAME_MAX_LENGTH) {
+		return { valid: false, error: SkillNameValidationError.TooLong }
+	}
+
+	if (!SKILL_NAME_REGEX.test(name)) {
+		return { valid: false, error: SkillNameValidationError.InvalidFormat }
+	}
+
+	return { valid: true }
+}

src/core/webview/__tests__/skillsMessageHandler.spec.ts

@@ -19,6 +19,19 @@ vi.mock("../../../integrations/misc/open-file", () => ({
 	openFile: vi.fn(),
 }))
 
+// Mock i18n
+vi.mock("../../../i18n", () => ({
+	t: (key: string, params?: Record<string, any>) => {
+		const translations: Record<string, string> = {
+			"skills:errors.missing_create_fields": "Missing required fields: skillName, source, or skillDescription",
+			"skills:errors.manager_unavailable": "Skills manager not available",
+			"skills:errors.missing_delete_fields": "Missing required fields: skillName or source",
+			"skills:errors.skill_not_found": `Skill "${params?.name}" not found`,
+		}
+		return translations[key] || key
+	},
+}))
+
 import * as vscode from "vscode"
 import { openFile } from "../../../integrations/misc/open-file"
 import { handleRequestSkills, handleCreateSkill, handleDeleteSkill, handleOpenSkillFile } from "../skillsMessageHandler"

src/core/webview/skillsMessageHandler.ts

@@ -4,6 +4,7 @@ import type { SkillMetadata, WebviewMessage } from "@roo-code/types"
 
 import type { ClineProvider } from "./ClineProvider"
 import { openFile } from "../../integrations/misc/open-file"
+import { t } from "../../i18n"
 
 /**
  * Handles the requestSkills message - returns all skills metadata
@@ -40,12 +41,12 @@ export async function handleCreateSkill(
 		const skillMode = message.skillMode
 
 		if (!skillName || !source || !skillDescription) {
-			throw new Error("Missing required fields: skillName, source, or skillDescription")
+			throw new Error(t("skills:errors.missing_create_fields"))
 		}
 
 		const skillsManager = provider.getSkillsManager()
 		if (!skillsManager) {
-			throw new Error("Skills manager not available")
+			throw new Error(t("skills:errors.manager_unavailable"))
 		}
 
 		const createdPath = await skillsManager.createSkill(skillName, source, skillDescription, skillMode)
@@ -78,12 +79,12 @@ export async function handleDeleteSkill(
 		const skillMode = message.skillMode
 
 		if (!skillName || !source) {
-			throw new Error("Missing required fields: skillName or source")
+			throw new Error(t("skills:errors.missing_delete_fields"))
 		}
 
 		const skillsManager = provider.getSkillsManager()
 		if (!skillsManager) {
-			throw new Error("Skills manager not available")
+			throw new Error(t("skills:errors.manager_unavailable"))
 		}
 
 		await skillsManager.deleteSkill(skillName, source, skillMode)
@@ -110,17 +111,17 @@ export async function handleOpenSkillFile(provider: ClineProvider, message: Webv
 		const skillMode = message.skillMode
 
 		if (!skillName || !source) {
-			throw new Error("Missing required fields: skillName or source")
+			throw new Error(t("skills:errors.missing_delete_fields"))
 		}
 
 		const skillsManager = provider.getSkillsManager()
 		if (!skillsManager) {
-			throw new Error("Skills manager not available")
+			throw new Error(t("skills:errors.manager_unavailable"))
 		}
 
 		const skill = skillsManager.getSkill(skillName, source, skillMode)
 		if (!skill) {
-			throw new Error(`Skill "${skillName}" not found`)
+			throw new Error(t("skills:errors.skill_not_found", { name: skillName }))
 		}
 
 		openFile(skill.path)

src/i18n/locales/ca/skills.json

@@ -0,0 +1,14 @@
+{
+	"errors": {
+		"name_length": "Skill name must be 1-{{maxLength}} characters (got {{length}})",
+		"name_format": "Skill name must be lowercase letters/numbers/hyphens only (no leading/trailing hyphen, no consecutive hyphens)",
+		"description_length": "Skill description must be 1-1024 characters (got {{length}})",
+		"no_workspace": "Cannot create project skill: no workspace folder is open",
+		"already_exists": "Skill \"{{name}}\" already exists at {{path}}",
+		"not_found": "Skill \"{{name}}\" not found in {{source}}{{modeInfo}}",
+		"missing_create_fields": "Missing required fields: skillName, source, or skillDescription",
+		"manager_unavailable": "Skills manager not available",
+		"missing_delete_fields": "Missing required fields: skillName or source",
+		"skill_not_found": "Skill \"{{name}}\" not found"
+	}
+}