fix(skills): report YAML frontmatter parse errors and serialize SKILL.md safely

SKILL.md files whose description contains unescaped double quotes failed to parse silently: gray-matter's YAMLException was swallowed by the outer catch and users only saw a misleading "missing required 'name' field" log (issue #859).

- Catch gray-matter parse errors separately in loadSkillMetadata and log the actual YAML syntax error, with a hint pointing at unescaped double quotes in the description line
- Build createSkill frontmatter as data and serialize via matter.stringify so special characters (double quotes, YAML booleans such as "yes", etc.) are quoted automatically and created skills always load
- Pass lineWidth: -1 to the dump options (with a typed alias for gray-matter's outdated options typings) to keep long plain scalars on one line and prevent updateSkillModes rewrites from reflowing values into folded block scalars
- Add regression tests covering both the load path (invalid YAML is skipped with the real cause logged, no misleading field error) and the create path (quoted frontmatter round-trips and loads on re-discovery)

Author: easonliang28

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

@@ -27,6 +27,7 @@ vi.mock("../../../i18n", () => ({
 			"skills:errors.manager_unavailable": "Skills manager not available",
 			"skills:errors.missing_delete_fields": "Missing required fields: skillName or source",
 			"skills:errors.missing_move_fields": "Missing required fields: skillName or source",
+			"skills:errors.missing_update_modes_fields": "Missing required fields: skillName or source",
 			"skills:errors.skill_not_found": `Skill "${params?.name}" not found`,
 		}
 		return translations[key] || key
@@ -41,6 +42,7 @@ import {
 	handleDeleteSkill,
 	handleMoveSkill,
 	handleOpenSkillFile,
+	handleUpdateSkillModes,
 } from "../skillsMessageHandler"
 
 describe("skillsMessageHandler", () => {
@@ -51,6 +53,7 @@ describe("skillsMessageHandler", () => {
 	const mockCreateSkill = vi.fn()
 	const mockDeleteSkill = vi.fn()
 	const mockMoveSkill = vi.fn()
+	const mockUpdateSkillModes = vi.fn()
 	const mockGetSkill = vi.fn()
 	const mockFindSkillByNameAndSource = vi.fn()
 
@@ -62,6 +65,7 @@ describe("skillsMessageHandler", () => {
 					createSkill: mockCreateSkill,
 					deleteSkill: mockDeleteSkill,
 					moveSkill: mockMoveSkill,
+					updateSkillModes: mockUpdateSkillModes,
 					getSkill: mockGetSkill,
 					findSkillByNameAndSource: mockFindSkillByNameAndSource,
 				}
@@ -384,6 +388,97 @@ describe("skillsMessageHandler", () => {
 		})
 	})
 
+	describe("handleUpdateSkillModes", () => {
+		it("updates a skill's mode slugs successfully", async () => {
+			const provider = createMockProvider(true)
+			mockUpdateSkillModes.mockResolvedValue(undefined)
+			mockGetSkillsMetadata.mockReturnValue([mockSkills[0]])
+
+			const result = await handleUpdateSkillModes(provider, {
+				type: "updateSkillModes",
+				skillName: "test-skill",
+				source: "global",
+				newSkillModeSlugs: ["code"],
+			} as WebviewMessage)
+
+			expect(result).toEqual([mockSkills[0]])
+			expect(mockUpdateSkillModes).toHaveBeenCalledWith("test-skill", "global", ["code"])
+			expect(mockPostMessageToWebview).toHaveBeenCalledWith({
+				type: "skills",
+				skills: [mockSkills[0]],
+				skillDiagnostics: [],
+			})
+		})
+
+		it("clears a skill's mode restriction with empty slugs", async () => {
+			const provider = createMockProvider(true)
+			mockUpdateSkillModes.mockResolvedValue(undefined)
+			mockGetSkillsMetadata.mockReturnValue([mockSkills[1]])
+
+			const result = await handleUpdateSkillModes(provider, {
+				type: "updateSkillModes",
+				skillName: "project-skill",
+				source: "project",
+				newSkillModeSlugs: [],
+			} as WebviewMessage)
+
+			expect(result).toEqual([mockSkills[1]])
+			expect(mockUpdateSkillModes).toHaveBeenCalledWith("project-skill", "project", [])
+		})
+
+		it("returns undefined when required fields are missing", async () => {
+			const provider = createMockProvider(true)
+
+			const result = await handleUpdateSkillModes(provider, {
+				type: "updateSkillModes",
+				skillName: "test-skill",
+				// missing source
+			} as WebviewMessage)
+
+			expect(result).toBeUndefined()
+			expect(mockUpdateSkillModes).not.toHaveBeenCalled()
+			expect(mockLog).toHaveBeenCalledWith(
+				"Error updating skill modes: Missing required fields: skillName or source",
+			)
+			expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
+				"Failed to update skill modes: Missing required fields: skillName or source",
+			)
+		})
+
+		it("returns undefined when skills manager is not available", async () => {
+			const provider = createMockProvider(false)
+
+			const result = await handleUpdateSkillModes(provider, {
+				type: "updateSkillModes",
+				skillName: "test-skill",
+				source: "global",
+				newSkillModeSlugs: ["code"],
+			} as WebviewMessage)
+
+			expect(result).toBeUndefined()
+			expect(mockLog).toHaveBeenCalledWith("Error updating skill modes: Skills manager not available")
+			expect(vscode.window.showErrorMessage).toHaveBeenCalledWith(
+				"Failed to update skill modes: Skills manager not available",
+			)
+		})
+
+		it("returns undefined and reports the error when updateSkillModes rejects", async () => {
+			const provider = createMockProvider(true)
+			mockUpdateSkillModes.mockRejectedValue(new Error("boom"))
+
+			const result = await handleUpdateSkillModes(provider, {
+				type: "updateSkillModes",
+				skillName: "test-skill",
+				source: "global",
+				newSkillModeSlugs: ["code"],
+			} as WebviewMessage)
+
+			expect(result).toBeUndefined()
+			expect(mockLog).toHaveBeenCalledWith("Error updating skill modes: boom")
+			expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("Failed to update skill modes: boom")
+		})
+	})
+
 	describe("handleOpenSkillFile", () => {
 		it("opens a skill file successfully", async () => {
 			const provider = createMockProvider(true)

src/services/skills/SkillsManager.ts

@@ -18,6 +18,33 @@ import { t } from "../../i18n"
 // Re-export for convenience
 export type { SkillMetadata, SkillContent, SkillDiagnostic }
 
+/**
+ * Extract the raw top-level `key: ...` line from the frontmatter block of a
+ * SKILL.md file. Used to point users at the exact line when YAML parsing
+ * fails (see issue #859). Returns undefined when the file has no
+ * frontmatter block or no matching top-level line.
+ */
+function getRawFrontmatterLine(fileContent: string, key: string): string | undefined {
+	const match = fileContent.match(/^---\r?\n([\s\S]*?)\r?\n---/)
+	if (!match) {
+		return undefined
+	}
+	const linePattern = new RegExp(`^${key}\\s*:`)
+	return match[1].split(/\r?\n/).find((line) => linePattern.test(line))
+}
+
+// gray-matter's bundled typings predate its options passthrough: `stringify`
+// forwards its options object to js-yaml's safeDump, so js-yaml dump options
+// such as `lineWidth` are honored at runtime even though the declared
+// `GrayMatterOption` interface does not list them. This type names the
+// supported subset explicitly (see issue #859).
+type SkillYamlDumpOptions = matter.GrayMatterOption<string, SkillYamlDumpOptions> & { lineWidth: number }
+
+// `lineWidth: -1` keeps long plain scalars (e.g. descriptions) on a single
+// line instead of reflowing them into folded block scalars, keeping SKILL.md
+// files stable across edits.
+const SKILL_YAML_DUMP_OPTIONS: SkillYamlDumpOptions = { lineWidth: -1 }
+
 export class SkillsManager {
 	private skills: Map<string, SkillMetadata> = new Map()
 	private diagnostics: SkillDiagnostic[] = []
@@ -104,11 +131,24 @@ export class SkillsManager {
 			const fileContent = await fs.readFile(skillMdPath, "utf-8")
 			let parsed: ReturnType<typeof matter>
 
+			// Parse errors are handled separately from field validation so that
+			// YAML syntax problems (e.g. unescaped double quotes in the
+			// description) report the actual cause instead of a misleading
+			// "missing required field" message (see issue #859).
 			try {
 				parsed = matter(fileContent)
 			} catch (error) {
 				this.recordDiagnostic(skillMdPath, source, error)
 				console.error(`Failed to parse skill at ${skillDir}:`, error)
+				// The most common cause is unescaped double quotes in the
+				// description value - point the user at the exact line.
+				const descriptionLine = getRawFrontmatterLine(fileContent, "description")
+				if (descriptionLine?.includes('"')) {
+					console.error(
+						`Hint: the "description" value in ${skillMdPath} contains unescaped double quotes. ` +
+							"Wrap the value in single quotes (or escape the double quotes) and save.",
+					)
+				}
 				return
 			}
 
@@ -422,6 +462,12 @@ export class SkillsManager {
 			.map((word) => word.charAt(0).toUpperCase() + word.slice(1))
 			.join(" ")
 
+		// Build the frontmatter as data and serialize it with gray-matter so
+		// values containing YAML special characters (e.g. double quotes in the
+		// description) are quoted/escaped automatically. Hand-built frontmatter
+		// produced unparseable SKILL.md files that silently failed to load
+		// (see issue #859). lineWidth: -1 keeps plain values on a single line,
+		// matching the previous output format.
 		const frontmatter = {
 			name,
 			description: trimmedDescription,
@@ -434,7 +480,7 @@ export class SkillsManager {
 
 Add your skill instructions here.
 `
-		const skillContent = matter.stringify(body, frontmatter)
+		const skillContent = matter.stringify(body, frontmatter, SKILL_YAML_DUMP_OPTIONS)
 
 		// Write the SKILL.md file
 		await fs.writeFile(skillMdPath, skillContent, "utf-8")
@@ -575,8 +621,10 @@ Add your skill instructions here.
 			delete frontmatter.mode
 		}
 
-		// Serialize back to SKILL.md format
-		const newContent = matter.stringify(body, frontmatter)
+		// Serialize back to SKILL.md format. lineWidth: -1 prevents long
+		// values from being reflowed into folded block scalars, keeping the
+		// file stable (see issue #859).
+		const newContent = matter.stringify(body, frontmatter, SKILL_YAML_DUMP_OPTIONS)
 		await fs.writeFile(skill.path, newContent, "utf-8")
 
 		// Refresh skills list