fix(skills): safely serialize skill frontmatter

packages/types/src/skills.ts

@@ -20,6 +20,15 @@ export interface SkillMetadata {
 	modeSlugs?: string[]
 }
 
+/** A user-actionable problem found while loading a SKILL.md file. */
+export interface SkillDiagnostic {
+	path: string
+	source: "global" | "project"
+	message: string
+	line?: number
+	column?: number
+}
+
 /**
  * Skill name validation constants per agentskills.io specification:
  * https://agentskills.io/specification

packages/types/src/vscode-extension-host.ts

@@ -14,7 +14,7 @@ import type { GitCommit } from "./git.js"
 import type { McpServer } from "./mcp.js"
 import type { ModelRecord, RouterModels } from "./model.js"
 import type { OpenAiCodexRateLimitInfo } from "./providers/openai-codex-rate-limits.js"
-import type { SkillMetadata } from "./skills.js"
+import type { SkillDiagnostic, SkillMetadata } from "./skills.js"
 import type { RuleMetadata } from "./rules.js"
 import type { TelemetrySetting } from "./telemetry.js"
 import type { WorktreeIncludeStatus } from "./worktree.js"
@@ -180,6 +180,7 @@ export interface ExtensionMessage {
 	list?: string[] // For dismissedUpsells
 	tools?: SerializedCustomToolDefinition[] // For customToolsResult
 	skills?: SkillMetadata[] // For skills response
+	skillDiagnostics?: SkillDiagnostic[] // For malformed skills omitted from the skills response
 	rules?: RuleMetadata[] // For rules response
 	modes?: { slug: string; name: string }[] // For modes response
 	rooHistoryImportProgress?: {

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

@@ -47,6 +47,7 @@ describe("skillsMessageHandler", () => {
 	const mockLog = vi.fn()
 	const mockPostMessageToWebview = vi.fn()
 	const mockGetSkillsMetadata = vi.fn()
+	const mockGetSkillDiagnostics = vi.fn()
 	const mockCreateSkill = vi.fn()
 	const mockDeleteSkill = vi.fn()
 	const mockMoveSkill = vi.fn()
@@ -57,6 +58,7 @@ describe("skillsMessageHandler", () => {
 		const skillsManager = hasSkillsManager
 			? {
 					getSkillsMetadata: mockGetSkillsMetadata,
+					getSkillDiagnostics: mockGetSkillDiagnostics,
 					createSkill: mockCreateSkill,
 					deleteSkill: mockDeleteSkill,
 					moveSkill: mockMoveSkill,
@@ -90,6 +92,7 @@ describe("skillsMessageHandler", () => {
 
 	beforeEach(() => {
 		vi.clearAllMocks()
+		mockGetSkillDiagnostics.mockReturnValue([])
 	})
 
 	describe("handleRequestSkills", () => {
@@ -100,7 +103,34 @@ describe("skillsMessageHandler", () => {
 			const result = await handleRequestSkills(provider)
 
 			expect(result).toEqual(mockSkills)
-			expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: mockSkills })
+			expect(mockPostMessageToWebview).toHaveBeenCalledWith({
+				type: "skills",
+				skills: mockSkills,
+				skillDiagnostics: [],
+			})
+		})
+
+		it("sends structured malformed-skill diagnostics without hiding valid skills", async () => {
+			const provider = createMockProvider(true)
+			const diagnostics = [
+				{
+					path: "/workspace/.roo/skills/broken/SKILL.md",
+					source: "project" as const,
+					message: "bad indentation of a mapping entry",
+					line: 3,
+					column: 20,
+				},
+			]
+			mockGetSkillsMetadata.mockReturnValue(mockSkills)
+			mockGetSkillDiagnostics.mockReturnValue(diagnostics)
+
+			await handleRequestSkills(provider)
+
+			expect(mockPostMessageToWebview).toHaveBeenCalledWith({
+				type: "skills",
+				skills: mockSkills,
+				skillDiagnostics: diagnostics,
+			})
 		})
 
 		it("returns empty skills when skills manager is not available", async () => {
@@ -109,7 +139,11 @@ describe("skillsMessageHandler", () => {
 			const result = await handleRequestSkills(provider)
 
 			expect(result).toEqual([])
-			expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: [] })
+			expect(mockPostMessageToWebview).toHaveBeenCalledWith({
+				type: "skills",
+				skills: [],
+				skillDiagnostics: [],
+			})
 		})
 
 		it("handles errors and returns empty skills", async () => {
@@ -122,7 +156,11 @@ describe("skillsMessageHandler", () => {
 
 			expect(result).toEqual([])
 			expect(mockLog).toHaveBeenCalled()
-			expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: [] })
+			expect(mockPostMessageToWebview).toHaveBeenCalledWith({
+				type: "skills",
+				skills: [],
+				skillDiagnostics: [],
+			})
 		})
 	})
 
@@ -142,7 +180,11 @@ describe("skillsMessageHandler", () => {
 			expect(result).toEqual(mockSkills)
 			expect(mockCreateSkill).toHaveBeenCalledWith("new-skill", "global", "New skill description", undefined)
 			expect(openFile).toHaveBeenCalledWith("/path/to/new-skill/SKILL.md")
-			expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: mockSkills })
+			expect(mockPostMessageToWebview).toHaveBeenCalledWith({
+				type: "skills",
+				skills: mockSkills,
+				skillDiagnostics: [],
+			})
 		})
 
 		it("creates a skill with mode restriction", async () => {
@@ -212,7 +254,11 @@ describe("skillsMessageHandler", () => {
 
 			expect(result).toEqual([mockSkills[1]])
 			expect(mockDeleteSkill).toHaveBeenCalledWith("test-skill", "global", undefined)
-			expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: [mockSkills[1]] })
+			expect(mockPostMessageToWebview).toHaveBeenCalledWith({
+				type: "skills",
+				skills: [mockSkills[1]],
+				skillDiagnostics: [],
+			})
 		})
 
 		it("deletes a skill with mode restriction", async () => {
@@ -280,7 +326,11 @@ describe("skillsMessageHandler", () => {
 
 			expect(result).toEqual([mockSkills[0]])
 			expect(mockMoveSkill).toHaveBeenCalledWith("test-skill", "global", undefined, "code")
-			expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: [mockSkills[0]] })
+			expect(mockPostMessageToWebview).toHaveBeenCalledWith({
+				type: "skills",
+				skills: [mockSkills[0]],
+				skillDiagnostics: [],
+			})
 		})
 
 		it("moves a skill from one mode to another", async () => {

src/core/webview/skillsMessageHandler.ts

@@ -16,15 +16,16 @@ export async function handleRequestSkills(provider: ClineProvider): Promise<Skil
 		const skillsManager = provider.getSkillsManager()
 		if (skillsManager) {
 			const skills = skillsManager.getSkillsMetadata()
-			await provider.postMessageToWebview({ type: "skills", skills })
+			const skillDiagnostics = skillsManager.getSkillDiagnostics()
+			await provider.postMessageToWebview({ type: "skills", skills, skillDiagnostics })
 			return skills
 		} else {
-			await provider.postMessageToWebview({ type: "skills", skills: [] })
+			await provider.postMessageToWebview({ type: "skills", skills: [], skillDiagnostics: [] })
 			return []
 		}
 	} catch (error) {
 		provider.log(`Error fetching skills: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`)
-		await provider.postMessageToWebview({ type: "skills", skills: [] })
+		await provider.postMessageToWebview({ type: "skills", skills: [], skillDiagnostics: [] })
 		return []
 	}
 }
@@ -59,7 +60,8 @@ export async function handleCreateSkill(
 
 		// Send updated skills list
 		const skills = skillsManager.getSkillsMetadata()
-		await provider.postMessageToWebview({ type: "skills", skills })
+		const skillDiagnostics = skillsManager.getSkillDiagnostics()
+		await provider.postMessageToWebview({ type: "skills", skills, skillDiagnostics })
 		return skills
 	} catch (error) {
 		const errorMessage = error instanceof Error ? error.message : String(error)
@@ -95,7 +97,8 @@ export async function handleDeleteSkill(
 
 		// Send updated skills list
 		const skills = skillsManager.getSkillsMetadata()
-		await provider.postMessageToWebview({ type: "skills", skills })
+		const skillDiagnostics = skillsManager.getSkillDiagnostics()
+		await provider.postMessageToWebview({ type: "skills", skills, skillDiagnostics })
 		return skills
 	} catch (error) {
 		const errorMessage = error instanceof Error ? error.message : String(error)
@@ -131,7 +134,8 @@ export async function handleMoveSkill(
 
 		// Send updated skills list
 		const skills = skillsManager.getSkillsMetadata()
-		await provider.postMessageToWebview({ type: "skills", skills })
+		const skillDiagnostics = skillsManager.getSkillDiagnostics()
+		await provider.postMessageToWebview({ type: "skills", skills, skillDiagnostics })
 		return skills
 	} catch (error) {
 		const errorMessage = error instanceof Error ? error.message : String(error)
@@ -166,7 +170,8 @@ export async function handleUpdateSkillModes(
 
 		// Send updated skills list
 		const skills = skillsManager.getSkillsMetadata()
-		await provider.postMessageToWebview({ type: "skills", skills })
+		const skillDiagnostics = skillsManager.getSkillDiagnostics()
+		await provider.postMessageToWebview({ type: "skills", skills, skillDiagnostics })
 		return skills
 	} catch (error) {
 		const errorMessage = error instanceof Error ? error.message : String(error)

src/services/skills/SkillsManager.ts

@@ -6,7 +6,7 @@ import matter from "gray-matter"
 import type { ClineProvider } from "../../core/webview/ClineProvider"
 import { getGlobalRooDirectory, getGlobalAgentsDirectory, getProjectAgentsDirectoryForCwd } from "../roo-config"
 import { directoryExists, fileExists } from "../roo-config"
-import { SkillMetadata, SkillContent } from "../../shared/skills"
+import { SkillMetadata, SkillContent, SkillDiagnostic } from "../../shared/skills"
 import { modes, getAllModes } from "../../shared/modes"
 import {
 	validateSkillName as validateSkillNameShared,
@@ -16,10 +16,11 @@ import {
 import { t } from "../../i18n"
 
 // Re-export for convenience
-export type { SkillMetadata, SkillContent }
+export type { SkillMetadata, SkillContent, SkillDiagnostic }
 
 export class SkillsManager {
 	private skills: Map<string, SkillMetadata> = new Map()
+	private diagnostics: SkillDiagnostic[] = []
 	private providerRef: WeakRef<ClineProvider>
 	private disposables: vscode.Disposable[] = []
 	private isDisposed = false
@@ -42,6 +43,7 @@ export class SkillsManager {
 	 */
 	async discoverSkills(): Promise<void> {
 		this.skills.clear()
+		this.diagnostics = []
 		const skillsDirs = await this.getSkillsDirectories()
 
 		for (const { dir, source, mode } of skillsDirs) {
@@ -100,9 +102,17 @@ export class SkillsManager {
 
 		try {
 			const fileContent = await fs.readFile(skillMdPath, "utf-8")
+			let parsed: ReturnType<typeof matter>
 
-			// Use gray-matter to parse frontmatter
-			const { data: frontmatter, content: body } = matter(fileContent)
+			try {
+				parsed = matter(fileContent)
+			} catch (error) {
+				this.recordDiagnostic(skillMdPath, source, error)
+				console.error(`Failed to parse skill at ${skillDir}:`, error)
+				return
+			}
+
+			const { data: frontmatter } = parsed
 
 			// Validate required fields (only name and description for now)
 			if (!frontmatter.name || typeof frontmatter.name !== "string") {
@@ -175,6 +185,20 @@ export class SkillsManager {
 		}
 	}
 
+	private recordDiagnostic(path: string, source: "global" | "project", error: unknown): void {
+		const yamlError = error as {
+			reason?: unknown
+			message?: unknown
+			mark?: { line?: unknown; column?: unknown }
+		}
+		const reason = typeof yamlError.reason === "string" ? yamlError.reason : undefined
+		const errorMessage = error instanceof Error ? error.message.split("\n", 1)[0] : String(error)
+		const line = typeof yamlError.mark?.line === "number" ? yamlError.mark.line + 1 : undefined
+		const column = typeof yamlError.mark?.column === "number" ? yamlError.mark.column + 1 : undefined
+
+		this.diagnostics.push({ path, source, message: reason ?? errorMessage, line, column })
+	}
+
 	/**
 	 * Get skills available for the current mode.
 	 * Resolves overrides: project > global, mode-specific > generic.
@@ -290,6 +314,10 @@ export class SkillsManager {
 		return this.getAllSkills()
 	}
 
+	getSkillDiagnostics(): SkillDiagnostic[] {
+		return [...this.diagnostics]
+	}
+
 	/**
 	 * Get a skill by name, source, and optionally mode
 	 */
@@ -394,25 +422,19 @@ export class SkillsManager {
 			.map((word) => word.charAt(0).toUpperCase() + word.slice(1))
 			.join(" ")
 
-		// Build frontmatter with optional modeSlugs
-		const frontmatterLines = [`name: ${name}`, `description: ${trimmedDescription}`]
-		if (modeSlugs && modeSlugs.length > 0) {
-			frontmatterLines.push(`modeSlugs:`)
-			for (const slug of modeSlugs) {
-				frontmatterLines.push(`  - ${slug}`)
-			}
+		const frontmatter = {
+			name,
+			description: trimmedDescription,
+			...(modeSlugs && modeSlugs.length > 0 ? { modeSlugs } : {}),
 		}
-
-		const skillContent = `---
-${frontmatterLines.join("\n")}
----
-
+		const body = `
 # ${titleName}
 
 ## Instructions
 
 Add your skill instructions here.
 `
+		const skillContent = matter.stringify(body, frontmatter)
 
 		// Write the SKILL.md file
 		await fs.writeFile(skillMdPath, skillContent, "utf-8")