Skip to content
This repository was archived by the owner on May 15, 2026. It is now read-only.

Commit e397c22

Browse files
author
xk
committed
fix: surface skill loading errors to users instead of silently dropping them
When skills fail validation during discovery (missing frontmatter fields, name/directory mismatch, invalid name format, bad description length), the errors were only logged to console.error() with no user-facing feedback. This change: - Adds SkillLoadWarning type to track failed skills with reasons - Collects warnings in SkillsManager during discoverSkills() - Exposes warnings via getLoadWarnings() method - Sends warnings to webview alongside skills metadata - Displays collapsible warning banner in Skills Settings UI - Adds 7 new tests for warning collection behavior Fixes #12194
1 parent ad25634 commit e397c22

8 files changed

Lines changed: 361 additions & 17 deletions

File tree

packages/types/src/skills.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@ export interface SkillMetadata {
2020
modeSlugs?: string[]
2121
}
2222

23+
/**
24+
* Warning emitted when a skill fails to load during discovery.
25+
* Collected by SkillsManager and surfaced to the UI so users know
26+
* which skills were skipped and why.
27+
*/
28+
export interface SkillLoadWarning {
29+
/** Directory name of the skill that failed to load */
30+
skillName: string
31+
/** Absolute path to the skill directory */
32+
path: string
33+
/** Whether this was a global or project skill */
34+
source: "global" | "project"
35+
/** Human-readable reason the skill was skipped */
36+
reason: string
37+
}
38+
2339
/**
2440
* Skill name validation constants per agentskills.io specification:
2541
* https://agentskills.io/specification

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import type { GitCommit } from "./git.js"
2020
import type { McpServer } from "./mcp.js"
2121
import type { ModelRecord, RouterModels } from "./model.js"
2222
import type { OpenAiCodexRateLimitInfo } from "./providers/openai-codex-rate-limits.js"
23-
import type { SkillMetadata } from "./skills.js"
23+
import type { SkillMetadata, SkillLoadWarning } from "./skills.js"
2424
import type { WorktreeIncludeStatus } from "./worktree.js"
2525

2626
/**
@@ -180,6 +180,7 @@ export interface ExtensionMessage {
180180
organizationId?: string | null // For organizationSwitchResult
181181
tools?: SerializedCustomToolDefinition[] // For customToolsResult
182182
skills?: SkillMetadata[] // For skills response
183+
skillLoadWarnings?: SkillLoadWarning[] // For skill load warnings
183184
modes?: { slug: string; name: string }[] // For modes response
184185
aggregatedCosts?: {
185186
// For taskWithAggregatedCosts response

src/core/webview/skillsMessageHandler.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as vscode from "vscode"
22

3-
import type { SkillMetadata, WebviewMessage } from "@roo-code/types"
3+
import type { SkillMetadata, SkillLoadWarning, WebviewMessage } from "@roo-code/types"
44

55
import type { ClineProvider } from "./ClineProvider"
66
import { openFile } from "../../integrations/misc/open-file"
@@ -16,7 +16,8 @@ export async function handleRequestSkills(provider: ClineProvider): Promise<Skil
1616
const skillsManager = provider.getSkillsManager()
1717
if (skillsManager) {
1818
const skills = skillsManager.getSkillsMetadata()
19-
await provider.postMessageToWebview({ type: "skills", skills })
19+
const skillLoadWarnings = skillsManager.getLoadWarnings()
20+
await provider.postMessageToWebview({ type: "skills", skills, skillLoadWarnings })
2021
return skills
2122
} else {
2223
await provider.postMessageToWebview({ type: "skills", skills: [] })

src/services/skills/SkillsManager.ts

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@ import {
1212
validateSkillName as validateSkillNameShared,
1313
SkillNameValidationError,
1414
SKILL_NAME_MAX_LENGTH,
15+
SkillLoadWarning,
1516
} from "@roo-code/types"
1617
import { t } from "../../i18n"
1718

1819
// Re-export for convenience
19-
export type { SkillMetadata, SkillContent }
20+
export type { SkillMetadata, SkillContent, SkillLoadWarning }
2021

2122
export class SkillsManager {
2223
private skills: Map<string, SkillMetadata> = new Map()
24+
private loadWarnings: SkillLoadWarning[] = []
2325
private providerRef: WeakRef<ClineProvider>
2426
private disposables: vscode.Disposable[] = []
2527
private isDisposed = false
@@ -42,6 +44,7 @@ export class SkillsManager {
4244
*/
4345
async discoverSkills(): Promise<void> {
4446
this.skills.clear()
47+
this.loadWarnings = []
4548
const skillsDirs = await this.getSkillsDirectories()
4649

4750
for (const { dir, source, mode } of skillsDirs) {
@@ -98,6 +101,8 @@ export class SkillsManager {
98101
const skillMdPath = path.join(skillDir, "SKILL.md")
99102
if (!(await fileExists(skillMdPath))) return
100103

104+
const effectiveSkillName = skillName || path.basename(skillDir)
105+
101106
try {
102107
const fileContent = await fs.readFile(skillMdPath, "utf-8")
103108

@@ -106,27 +111,34 @@ export class SkillsManager {
106111

107112
// Validate required fields (only name and description for now)
108113
if (!frontmatter.name || typeof frontmatter.name !== "string") {
109-
console.error(`Skill at ${skillDir} is missing required 'name' field`)
114+
const reason = `Missing required 'name' field in frontmatter`
115+
console.error(`Skill at ${skillDir}: ${reason}`)
116+
this.loadWarnings.push({ skillName: effectiveSkillName, path: skillDir, source, reason })
110117
return
111118
}
112119
if (!frontmatter.description || typeof frontmatter.description !== "string") {
113-
console.error(`Skill at ${skillDir} is missing required 'description' field`)
120+
const reason = `Missing required 'description' field in frontmatter`
121+
console.error(`Skill at ${skillDir}: ${reason}`)
122+
this.loadWarnings.push({ skillName: effectiveSkillName, path: skillDir, source, reason })
114123
return
115124
}
116125

117126
// Validate that frontmatter name matches the skill name (directory name or symlink name)
118127
// Per the Agent Skills spec: "name field must match the parent directory name"
119-
const effectiveSkillName = skillName || path.basename(skillDir)
120128
if (frontmatter.name !== effectiveSkillName) {
121-
console.error(`Skill name "${frontmatter.name}" doesn't match directory "${effectiveSkillName}"`)
129+
const reason = `Frontmatter name "${frontmatter.name}" doesn't match directory name "${effectiveSkillName}"`
130+
console.error(`Skill at ${skillDir}: ${reason}`)
131+
this.loadWarnings.push({ skillName: effectiveSkillName, path: skillDir, source, reason })
122132
return
123133
}
124134

125135
// Validate skill name per agentskills.io spec using shared validation
126136
const nameValidation = validateSkillNameShared(effectiveSkillName)
127137
if (!nameValidation.valid) {
128138
const errorMessage = this.getSkillNameErrorMessage(effectiveSkillName, nameValidation.error!)
129-
console.error(`Skill name "${effectiveSkillName}" is invalid: ${errorMessage}`)
139+
const reason = `Invalid skill name: ${errorMessage}`
140+
console.error(`Skill "${effectiveSkillName}": ${reason}`)
141+
this.loadWarnings.push({ skillName: effectiveSkillName, path: skillDir, source, reason })
130142
return
131143
}
132144

@@ -135,9 +147,9 @@ export class SkillsManager {
135147
// - non-empty (after trimming)
136148
const description = frontmatter.description.trim()
137149
if (description.length < 1 || description.length > 1024) {
138-
console.error(
139-
`Skill "${effectiveSkillName}" has an invalid description length: must be 1-1024 characters (got ${description.length})`,
140-
)
150+
const reason = `Invalid description length: must be 1-1024 characters (got ${description.length})`
151+
console.error(`Skill "${effectiveSkillName}": ${reason}`)
152+
this.loadWarnings.push({ skillName: effectiveSkillName, path: skillDir, source, reason })
141153
return
142154
}
143155

@@ -171,7 +183,9 @@ export class SkillsManager {
171183
modeSlugs, // New: array of mode slugs, undefined = any mode
172184
})
173185
} catch (error) {
174-
console.error(`Failed to load skill at ${skillDir}:`, error)
186+
const reason = `Failed to load skill: ${error instanceof Error ? error.message : String(error)}`
187+
console.error(`Skill at ${skillDir}: ${reason}`)
188+
this.loadWarnings.push({ skillName: effectiveSkillName, path: skillDir, source, reason })
175189
}
176190
}
177191

@@ -258,6 +272,14 @@ export class SkillsManager {
258272
return Array.from(this.skills.values())
259273
}
260274

275+
/**
276+
* Get warnings collected during the last skill discovery.
277+
* Returns skills that failed to load with their specific error reasons.
278+
*/
279+
getLoadWarnings(): SkillLoadWarning[] {
280+
return [...this.loadWarnings]
281+
}
282+
261283
async getSkillContent(name: string, currentMode?: string): Promise<SkillContent | null> {
262284
// If mode is provided, try to find the best matching skill
263285
let skill: SkillMetadata | undefined
@@ -715,5 +737,6 @@ Add your skill instructions here.
715737
this.disposables.forEach((d) => d.dispose())
716738
this.disposables = []
717739
this.skills.clear()
740+
this.loadWarnings = []
718741
}
719742
}

0 commit comments

Comments
 (0)