diff --git a/.changeset/skill-load-warning.md b/.changeset/skill-load-warning.md new file mode 100644 index 0000000000..4dafada185 --- /dev/null +++ b/.changeset/skill-load-warning.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/agent-core": patch +"@moonshot-ai/kimi-code": patch +--- + +Surface a warning when a skill file fails to parse at session start, instead of dropping it silently. The warning names the offending file and appears in the TUI status line. diff --git a/packages/agent-core/src/session/index.ts b/packages/agent-core/src/session/index.ts index 63f8d6152e..d9d32fbbc7 100644 --- a/packages/agent-core/src/session/index.ts +++ b/packages/agent-core/src/session/index.ts @@ -694,6 +694,14 @@ export class Session { severity: 'warning', }); } + await this.skillsReady; + for (const warning of this.skills.getLoadWarnings()) { + warnings.push({ + code: 'skill-load-failed', + message: warning.message, + severity: 'warning', + }); + } return warnings; } diff --git a/packages/agent-core/src/skill/registry.ts b/packages/agent-core/src/skill/registry.ts index 65b207e274..6a655c35b8 100644 --- a/packages/agent-core/src/skill/registry.ts +++ b/packages/agent-core/src/skill/registry.ts @@ -23,11 +23,16 @@ export interface SkillRegistryOptions { readonly sessionId?: string; } +export interface SkillLoadWarning { + readonly message: string; +} + export class SessionSkillRegistry implements AgentSkillRegistry { private readonly byName = new Map(); private readonly byPluginAndName = new Map(); private readonly roots: string[] = []; private readonly skipped: SkippedSkill[] = []; + private readonly warnings: SkillLoadWarning[] = []; private readonly discoverImpl: typeof discoverSkills; private readonly onWarning: (message: string, cause?: unknown) => void; readonly sessionId?: string; @@ -45,7 +50,10 @@ export class SessionSkillRegistry implements AgentSkillRegistry { const skills = await this.discoverImpl({ roots, - onWarning: this.onWarning, + onWarning: (message, cause) => { + this.warnings.push({ message }); + this.onWarning(message, cause); + }, onSkippedByPolicy: (skill) => this.skipped.push(skill), onDiscoveredSkill: (skill) => { this.indexPluginSkill(skill); @@ -125,6 +133,10 @@ export class SessionSkillRegistry implements AgentSkillRegistry { return [...this.skipped]; } + getLoadWarnings(): readonly SkillLoadWarning[] { + return [...this.warnings]; + } + getKimiSkillsDescription(): string { const rendered = renderGroupedSkills(this.listSkills(), formatFullSkill); return rendered.length === 0 ? 'No skills' : rendered; diff --git a/packages/agent-core/test/skill/registry.test.ts b/packages/agent-core/test/skill/registry.test.ts index 7e9e26056b..cd494f76b6 100644 --- a/packages/agent-core/test/skill/registry.test.ts +++ b/packages/agent-core/test/skill/registry.test.ts @@ -1,4 +1,8 @@ -import { describe, expect, it } from 'vitest'; +import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'pathe'; + +import { afterEach, describe, expect, it } from 'vitest'; import { SessionSkillRegistry } from '../../src/skill'; import type { SkillDefinition, SkillSource } from '../../src/skill'; @@ -161,3 +165,97 @@ function sectionFor(rendered: string, header: string): string { const next = rendered.indexOf('### ', start + header.length); return next === -1 ? rendered.slice(start) : rendered.slice(start, next); } + +const tempDirs: string[] = []; + +afterEach(async () => { + for (const dir of tempDirs.splice(0)) { + await rm(dir, { recursive: true, force: true }); + } +}); + +describe('SessionSkillRegistry load warnings', () => { + it('collects a warning when a skill file fails to parse', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'kimi-skill-registry-')); + tempDirs.push(root); + // Directory-form SKILL.md with a missing required "name" field triggers a + // SkillParseError (not an unsupported-type skip), which must surface as a + // load warning instead of being silently dropped. + await writeSkill(root, path.join('broken', 'SKILL.md'), [ + '---', + 'description: missing the name field', + '---', + 'body', + ]); + + const registry = new SessionSkillRegistry(); + await registry.loadRoots([{ path: root, source: 'project' }]); + + expect(registry.listSkills()).toEqual([]); + const warnings = registry.getLoadWarnings(); + expect(warnings.length).toBeGreaterThan(0); + expect(warnings.some((w) => w.message.includes('broken'))).toBe(true); + }); + + it('collects a warning for malformed YAML frontmatter', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'kimi-skill-registry-')); + tempDirs.push(root); + // "description: foo: bar" is not a valid YAML plain scalar — the second + // ": " turns the line into an ambiguous mapping, which js-yaml rejects. + await writeSkill(root, path.join('bad-yaml', 'SKILL.md'), [ + '---', + 'name: bad-yaml', + 'description: foo: bar', + '---', + 'body', + ]); + + const registry = new SessionSkillRegistry(); + await registry.loadRoots([{ path: root, source: 'user' }]); + + expect(registry.listSkills()).toEqual([]); + expect(registry.getLoadWarnings().length).toBeGreaterThan(0); + }); + + it('produces no warning when all skills parse cleanly', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'kimi-skill-registry-')); + tempDirs.push(root); + await writeSkill(root, path.join('good', 'SKILL.md'), [ + '---', + 'name: good', + 'description: A fine skill', + '---', + 'body', + ]); + + const registry = new SessionSkillRegistry(); + await registry.loadRoots([{ path: root, source: 'project' }]); + + expect(registry.listSkills().map((s) => s.name)).toEqual(['good']); + expect(registry.getLoadWarnings()).toEqual([]); + }); + + it('still forwards warnings to the onWarning callback', async () => { + const root = await mkdtemp(path.join(tmpdir(), 'kimi-skill-registry-')); + tempDirs.push(root); + await writeSkill(root, path.join('broken', 'SKILL.md'), [ + '---', + 'description: missing the name field', + '---', + 'body', + ]); + + const received: string[] = []; + const registry = new SessionSkillRegistry({ onWarning: (m) => received.push(m) }); + await registry.loadRoots([{ path: root, source: 'project' }]); + + expect(registry.getLoadWarnings().length).toBeGreaterThan(0); + expect(received.length).toBeGreaterThan(0); + }); +}); + +async function writeSkill(root: string, relativePath: string, lines: readonly string[]): Promise { + const target = path.join(root, relativePath); + await mkdir(path.dirname(target), { recursive: true }); + await writeFile(target, lines.join('\n')); +}