fix(skills): safely serialize skill frontmatter

[taltas]

Summary

• Safely serialize generated skill frontmatter as YAML so descriptions containing quotes or other YAML-sensitive characters remain valid.
• Surface visible diagnostics for malformed skills instead of silently omitting them or showing a misleading missing-name error.

Root cause and user impact

Skill creation interpolated description text directly into double-quoted YAML frontmatter. Descriptions containing unescaped double quotes could therefore produce invalid SKILL.md files, causing frontmatter parsing to fail even when the required name field was present. The parsing failure was then obscured by misleading or absent diagnostics, leaving affected skills unavailable without explaining the actual malformed YAML.

This change uses safe YAML serialization for generated frontmatter and preserves actionable parse diagnostics so users can identify and correct malformed skill files.

Testing and validation

• Backend skill test suites: 74 tests passed.
• Webview skill test suites: 20 tests passed.
• Package typechecks passed.
• Lint and formatting checks passed.
• No-mistakes run 01KXSJBKY4H2SA0KV63NCNQ210 completed with no findings.
• Push-time repository typechecks passed.

Fixes #859

Summary by CodeRabbit

• New Features

  • Added diagnostics for malformed skill configuration files, including file location and error details.
  • Skills that fail to load no longer prevent valid skills from appearing.
  • Added a warning panel in Skills settings with actionable troubleshooting information.
  • Improved skill creation to preserve descriptions and mode settings reliably.

• Bug Fixes

  • Improved handling of YAML-sensitive characters in skill metadata.

[coderabbitai]

📝 Walkthrough

Walkthrough

Skill loading now records malformed frontmatter diagnostics while retaining valid skills. Diagnostics flow through extension messages into webview state and appear in the Skills settings UI with localized guidance and source locations.

Changes

Skill diagnostics discovery

Layer / File(s)	Summary
Diagnostic contract and discovery packages/types/src/skills.ts, src/shared/skills.ts, src/services/skills/SkillsManager.ts, src/services/skills/__tests__/SkillsManager.spec.ts	Adds the SkillDiagnostic type, records YAML parsing failures with optional line and column data, exposes diagnostics, and validates frontmatter serialization and discovery behavior.

Webview propagation

Layer / File(s)	Summary
Diagnostic message propagation packages/types/src/vscode-extension-host.ts, src/core/webview/skillsMessageHandler.ts, src/core/webview/__tests__/skillsMessageHandler.spec.ts	Includes diagnostics in skills messages for requests and skill mutations, with coverage for successful, empty, and error cases.

Settings display

Layer / File(s)	Summary
Settings diagnostics display webview-ui/src/context/ExtensionStateContext.tsx, webview-ui/src/components/settings/SkillsSettings.tsx, webview-ui/src/components/settings/__tests__/SkillsSettings.spec.tsx, webview-ui/src/i18n/locales/en/settings.json	Stores incoming diagnostics in extension state and renders localized warning details with file, line, column, and message information.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SkillsManager
  participant skillsMessageHandler
  participant ExtensionStateContext
  participant SkillsSettings
  SkillsManager->>skillsMessageHandler: provide skills and diagnostics
  skillsMessageHandler->>ExtensionStateContext: send skills message
  ExtensionStateContext->>ExtensionStateContext: update diagnostic state
  ExtensionStateContext->>SkillsSettings: expose skillDiagnostics
  SkillsSettings->>SkillsSettings: render localized warning details

Loading

Suggested labels: awaiting-review

Suggested reviewers: navedmerchant, hannesrudolph, edelauna

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and clearly describes the main change around safer skill frontmatter serialization.
Description check	✅ Passed	The description covers the issue, root cause, implementation, and validation, though it doesn't follow the template headings exactly.
Linked Issues check	✅ Passed	The changes address #859 by safely serializing skill YAML and surfacing parse diagnostics instead of hiding malformed files.
Out of Scope Changes check	✅ Passed	The diff stays focused on skill frontmatter serialization, diagnostics plumbing, UI display, and tests for the linked bug.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-859-skill-yaml

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/services/skills/SkillsManager.ts (1)

115-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Surface schema and validation errors as diagnostics.

Currently, if a SKILL.md is valid YAML but fails schema validation (e.g., missing required fields, name mismatch, or invalid description length), it is skipped and only logged to the console. Since the diagnostics UI pipeline is now in place, consider recording these validation errors as SkillDiagnostics as well to give users actionable feedback instead of letting the skill silently disappear.

💡 Proposed refactor to record validation diagnostics

 			const { data: frontmatter } = parsed
 
 			// Validate required fields (only name and description for now)
 			if (!frontmatter.name || typeof frontmatter.name !== "string") {
+				this.recordDiagnostic(skillMdPath, source, `Missing required 'name' field in frontmatter`)
 				console.error(`Skill at ${skillDir} is missing required 'name' field`)
 				return
 			}
 			if (!frontmatter.description || typeof frontmatter.description !== "string") {
+				this.recordDiagnostic(skillMdPath, source, `Missing required 'description' field in frontmatter`)
 				console.error(`Skill at ${skillDir} is missing required 'description' field`)
 				return
 			}
 
 			// Validate that frontmatter name matches the skill name (directory name or symlink name)
 			// Per the Agent Skills spec: "name field must match the parent directory name"
 			const effectiveSkillName = skillName || path.basename(skillDir)
 			if (frontmatter.name !== effectiveSkillName) {
+				this.recordDiagnostic(skillMdPath, source, `Skill name "${frontmatter.name}" doesn't match directory "${effectiveSkillName}"`)
 				console.error(`Skill name "${frontmatter.name}" doesn't match directory "${effectiveSkillName}"`)
 				return
 			}
 
 			// Validate skill name per agentskills.io spec using shared validation
 			const nameValidation = validateSkillNameShared(effectiveSkillName)
 			if (!nameValidation.valid) {
 				const errorMessage = this.getSkillNameErrorMessage(effectiveSkillName, nameValidation.error!)
+				this.recordDiagnostic(skillMdPath, source, `Skill name "${effectiveSkillName}" is invalid: ${errorMessage}`)
 				console.error(`Skill name "${effectiveSkillName}" is invalid: ${errorMessage}`)
 				return
 			}
 
 			// Description constraints:
 			// - 1-1024 chars
 			// - non-empty (after trimming)
 			const description = frontmatter.description.trim()
 			if (description.length < 1 || description.length > 1024) {
+				this.recordDiagnostic(skillMdPath, source, `Invalid description length: must be 1-1024 characters (got ${description.length})`)
 				console.error(
 					`Skill "${effectiveSkillName}" has an invalid description length: must be 1-1024 characters (got ${description.length})`,
 				)
 				return
 			}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/services/skills/SkillsManager.ts` around lines 115 - 152, Update the
SKILL.md validation flow in SkillsManager to create and record a SkillDiagnostic
for every schema validation failure before returning, including missing name,
missing description, name mismatch, invalid name, and invalid description
length. Preserve the existing console logging and skill-skipping behavior, and
route each diagnostic through the existing diagnostics pipeline rather than only
logging it.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/services/skills/SkillsManager.ts`:
- Around line 115-152: Update the SKILL.md validation flow in SkillsManager to
create and record a SkillDiagnostic for every schema validation failure before
returning, including missing name, missing description, name mismatch, invalid
name, and invalid description length. Preserve the existing console logging and
skill-skipping behavior, and route each diagnostic through the existing
diagnostics pipeline rather than only logging it.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 382fe9b7-e5a9-4a40-954d-674903cdde2c

📥 Commits

Reviewing files that changed from the base of the PR and between 3a4006a and e9eed2e.

📒 Files selected for processing (11)

• packages/types/src/skills.ts
• packages/types/src/vscode-extension-host.ts
• src/core/webview/__tests__/skillsMessageHandler.spec.ts
• src/core/webview/skillsMessageHandler.ts
• src/services/skills/SkillsManager.ts
• src/services/skills/__tests__/SkillsManager.spec.ts
• src/shared/skills.ts
• webview-ui/src/components/settings/SkillsSettings.tsx
• webview-ui/src/components/settings/__tests__/SkillsSettings.spec.tsx
• webview-ui/src/context/ExtensionStateContext.tsx
• webview-ui/src/i18n/locales/en/settings.json