From 774dac7f27d9bf9acb8c73c0eba6fc7e4014f07c Mon Sep 17 00:00:00 2001 From: liukx0205 <69756503+liukx0205@users.noreply.github.com> Date: Wed, 9 Sep 2026 08:37:09 +0000 Subject: [PATCH 1/2] fix(skills): surface invalid SKILL.md files with their skip reasons Skill discovery recorded only unsupported-type skips; parse failures (missing frontmatter, invalid YAML, missing required fields) were logged as warnings that daemon users never see, so a broken SKILL.md looked exactly like a hot-reload no-op. Record parse failures as skipped entries (path, type, reason) in both discovery implementations and expose them as invalid_skills in the session and workspace skill-list responses. Fixes #3673 --- .changeset/invalid-skill-visibility.md | 5 ++ .../skill/catalog/fileSkillDiscovery.ts | 5 ++ .../skill/workspace/runtimeSkillDiscovery.ts | 5 ++ .../skill/catalog/fileSkillDiscovery.test.ts | 32 +++++++++ .../kap-server/src/protocol/rest-skill.ts | 8 +++ packages/kap-server/src/routes/skills.ts | 25 ++++--- packages/kap-server/test/skills.test.ts | 72 +++++++++++++++++++ 7 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 .changeset/invalid-skill-visibility.md diff --git a/.changeset/invalid-skill-visibility.md b/.changeset/invalid-skill-visibility.md new file mode 100644 index 00000000000..c3e09e35edb --- /dev/null +++ b/.changeset/invalid-skill-visibility.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Surface skill files that fail to parse, with the file path and failure reason, in the daemon skill listings. diff --git a/packages/agent-core-v2/src/features/skill/catalog/fileSkillDiscovery.ts b/packages/agent-core-v2/src/features/skill/catalog/fileSkillDiscovery.ts index 95ce5c01d28..82b054bdd7d 100644 --- a/packages/agent-core-v2/src/features/skill/catalog/fileSkillDiscovery.ts +++ b/packages/agent-core-v2/src/features/skill/catalog/fileSkillDiscovery.ts @@ -195,6 +195,11 @@ async function parseAndRegister(input: { reason: `unsupported skill type "${error.skillType}"`, }); } else if (error instanceof SkillParseError) { + input.skipped.push({ + path: input.skillMdPath, + type: 'invalid', + reason: error.message, + }); input.warn?.(`Skipping invalid skill at ${input.skillMdPath}: ${error.message}`, error); } else { input.warn?.(`Skipping skill at ${input.skillMdPath} due to unexpected error`, error); diff --git a/packages/agent-core-v2/src/features/skill/workspace/runtimeSkillDiscovery.ts b/packages/agent-core-v2/src/features/skill/workspace/runtimeSkillDiscovery.ts index 602ff185a0b..af99a738825 100644 --- a/packages/agent-core-v2/src/features/skill/workspace/runtimeSkillDiscovery.ts +++ b/packages/agent-core-v2/src/features/skill/workspace/runtimeSkillDiscovery.ts @@ -66,6 +66,11 @@ async function discoverRuntimeSkills( reason: `unsupported skill type "${error.skillType}"`, }); } else if (error instanceof SkillParseError) { + skipped.push({ + path: input.skillMdPath, + type: 'invalid', + reason: error.message, + }); warn?.(`Skipping invalid skill at ${input.skillMdPath}: ${error.message}`, error); } else { warn?.(`Skipping skill at ${input.skillMdPath} due to unexpected error`, error); diff --git a/packages/agent-core-v2/test/features/skill/catalog/fileSkillDiscovery.test.ts b/packages/agent-core-v2/test/features/skill/catalog/fileSkillDiscovery.test.ts index 6b9136939c4..6f0205f2e82 100644 --- a/packages/agent-core-v2/test/features/skill/catalog/fileSkillDiscovery.test.ts +++ b/packages/agent-core-v2/test/features/skill/catalog/fileSkillDiscovery.test.ts @@ -241,6 +241,13 @@ describe('FileSkillDiscovery', () => { const result = await discover([skillRoot('skills')]); expect(result.skills).toEqual([]); + expect(result.skipped).toEqual([ + { + path: skillMdPath, + type: 'invalid', + reason: `Missing frontmatter in ${skillMdPath}`, + }, + ]); expect(warnings).toEqual([ { message: `Skipping invalid skill at ${skillMdPath}: Missing frontmatter in ${skillMdPath}`, @@ -249,6 +256,31 @@ describe('FileSkillDiscovery', () => { ]); }); + it('records unparsable YAML and missing required fields as skipped with their reasons', async () => { + const badYamlPath = join(root, 'skills/bad-yaml/SKILL.md'); + await mkdir(dirname(badYamlPath), { recursive: true }); + await writeFile(badYamlPath, '---\nname: [unclosed\n---\nbody'); + const noDescriptionPath = join(root, 'skills/no-description/SKILL.md'); + await mkdir(dirname(noDescriptionPath), { recursive: true }); + await writeFile(noDescriptionPath, '---\nname: no-description\n---\nbody'); + + const result = await discover([skillRoot('skills')]); + + expect(result.skills).toEqual([]); + expect(result.skipped).toEqual([ + { + path: badYamlPath, + type: 'invalid', + reason: expect.stringContaining(`Invalid frontmatter in ${badYamlPath}`), + }, + { + path: noDescriptionPath, + type: 'invalid', + reason: `Missing required frontmatter field "description" in ${noDescriptionPath}`, + }, + ]); + }); + it('records a skill with an unsupported type as skipped instead of warning', async () => { await writeSkill('skills/legacy/SKILL.md', 'name: legacy\ndescription: old\ntype: nope'); diff --git a/packages/kap-server/src/protocol/rest-skill.ts b/packages/kap-server/src/protocol/rest-skill.ts index fa940e60fd3..89517e0b035 100644 --- a/packages/kap-server/src/protocol/rest-skill.ts +++ b/packages/kap-server/src/protocol/rest-skill.ts @@ -3,8 +3,16 @@ import { z } from 'zod'; import { fileContentSchema, imageContentSchema, videoContentSchema } from './message'; import { skillDescriptorSchema } from './skill'; +export const invalidSkillSchema = z.object({ + path: z.string(), + type: z.string(), + reason: z.string(), +}); +export type InvalidSkill = z.infer; + export const listSkillsResponseSchema = z.object({ skills: z.array(skillDescriptorSchema), + invalid_skills: z.array(invalidSkillSchema), }); export type ListSkillsResponse = z.infer; diff --git a/packages/kap-server/src/routes/skills.ts b/packages/kap-server/src/routes/skills.ts index 2c5dcae0fd2..cbeac927854 100644 --- a/packages/kap-server/src/routes/skills.ts +++ b/packages/kap-server/src/routes/skills.ts @@ -57,6 +57,7 @@ import { activateSkillRequestSchema, activateSkillResultSchema, listSkillsResponseSchema, + type InvalidSkill, } from '../protocol/rest-skill'; import { workspaceIdParamSchema } from '../protocol/rest-workspace'; import type { SkillDescriptor } from '../protocol/skill'; @@ -120,7 +121,8 @@ export function registerSkillsRoutes(app: SkillsRouteHost, core: Scope): void { errors: { [ErrorCode.SESSION_NOT_FOUND]: {}, }, - description: 'List the skills available to a session', + description: + 'List the skills available to a session, plus the skill files discovery skipped as invalid (invalid_skills: path, type, reason)', tags: ['skills'], operationId: 'listSkills', }, @@ -134,7 +136,8 @@ export function registerSkillsRoutes(app: SkillsRouteHost, core: Scope): void { const catalog = resolved.handle.accessor.get(ISessionSkillCatalog); await catalog.ready; const skills = catalog.catalog.listSkills().map(toProtocolSkill); - reply.send(okEnvelope({ skills }, req.id)); + const invalid = catalog.catalog.getSkippedByPolicy(); + reply.send(okEnvelope({ skills, invalid_skills: invalid }, req.id)); }, ); app.get( @@ -152,7 +155,8 @@ export function registerSkillsRoutes(app: SkillsRouteHost, core: Scope): void { errors: { [ErrorCode.WORKSPACE_NOT_FOUND]: {}, }, - description: 'List the skills available to a workspace (no session required)', + description: + 'List the skills available to a workspace (no session required), plus the skill files discovery skipped as invalid (invalid_skills: path, type, reason)', tags: ['skills'], operationId: 'listWorkspaceSkills', }, @@ -169,8 +173,10 @@ export function registerSkillsRoutes(app: SkillsRouteHost, core: Scope): void { ); return; } - const skills = (await listWorkspaceSkillsForRoot(core, ws.root)).map(toProtocolSkill); - reply.send(okEnvelope({ skills }, req.id)); + const { skills, invalid } = await listWorkspaceSkillsForRoot(core, ws.root); + reply.send( + okEnvelope({ skills: skills.map(toProtocolSkill), invalid_skills: invalid }, req.id), + ); }, ); app.get( @@ -285,9 +291,9 @@ export function registerSkillsRoutes(app: SkillsRouteHost, core: Scope): void { preparedMedia = undefined; requestLog(req)?.info({ session_id, skill_name: parsed.id }, 'skill activated'); reply.send(okEnvelope({ activated: true, skill_name: parsed.id }, req.id)); - } catch (err) { + } catch (error) { await preparedMedia?.discard(); - sendMappedError(reply, req.id, err); + sendMappedError(reply, req.id, error); } }, ); @@ -301,7 +307,7 @@ export function registerSkillsRoutes(app: SkillsRouteHost, core: Scope): void { async function listWorkspaceSkillsForRoot( core: Scope, workDir: string, -): Promise { +): Promise<{ readonly skills: readonly SkillDefinition[]; readonly invalid: readonly InvalidSkill[] }> { const discovery = core.accessor.get(ISkillDiscovery); const bootstrap = core.accessor.get(IBootstrapService); const plugins = core.accessor.get(IPluginService); @@ -347,7 +353,8 @@ async function listWorkspaceSkillsForRoot( for (const { skills } of ordered) { for (const skill of skills) catalog.register(skill, { replace: true }); } - return catalog.listSkills(); + const invalid = [user, project, explicit, extra, plugin].flatMap((result) => [...result.skipped]); + return { skills: catalog.listSkills(), invalid }; } type SkillElement = ReturnType[number]; diff --git a/packages/kap-server/test/skills.test.ts b/packages/kap-server/test/skills.test.ts index f4b8c1db707..dc0a5699b59 100644 --- a/packages/kap-server/test/skills.test.ts +++ b/packages/kap-server/test/skills.test.ts @@ -112,6 +112,12 @@ describe('server-v2 /api/v1 skills', () => { ); } + async function seedInvalidProjectSkill(root: string, name: string, content: string): Promise { + const dir = join(root, '.kimi-code', 'skills', name); + await mkdir(dir, { recursive: true }); + await writeFile(join(dir, 'SKILL.md'), content); + } + async function seedExplicitSkill(root: string, name: string): Promise { const dir = join(root, name); await mkdir(dir, { recursive: true }); @@ -167,6 +173,27 @@ describe('server-v2 /api/v1 skills', () => { expect(docsSkill).toMatchObject({ source: 'builtin' }); expect(docsSkill?.description.length).toBeGreaterThan(0); }); + + it('surfaces invalid project skill files with their skip reasons', async () => { + const workspaceDir = await makeWorkspaceDir(); + await seedInvalidProjectSkill(workspaceDir, 'broken', 'no frontmatter here'); + const id = await createSession(workspaceDir); + + const { body } = await getJson<{ skills: SkillWire[] }>( + `/api/v1/sessions/${id}/skills`, + ); + expect(body.code).toBe(0); + const parsed = listSkillsResponseSchema.parse(body.data); + expect(parsed.skills.some((s) => s.name === 'broken')).toBe(false); + const invalid = parsed.invalid_skills.filter((s) => s.path.startsWith(workspaceDir)); + expect(invalid).toEqual([ + { + path: join(workspaceDir, '.kimi-code', 'skills', 'broken', 'SKILL.md'), + type: 'invalid', + reason: expect.stringContaining('Missing frontmatter'), + }, + ]); + }); }); describe('POST /api/v1/sessions/{sid}/skills/{name}:activate', () => { @@ -475,5 +502,50 @@ describe('server-v2 /api/v1 skills', () => { ); expect(body.code).toBe(40410); }); + + it('surfaces invalid skill files with path and reason for each failure kind', async () => { + await server!.close(); + server = undefined; + server = await startServer({ + hostIdentity: TEST_HOST_IDENTITY, + host: '127.0.0.1', + port: 0, + homeDir: home, + logLevel: 'silent', + }); + base = `http://127.0.0.1:${server.port}`; + + const workspaceDir = await makeWorkspaceDir(); + await seedInvalidProjectSkill(workspaceDir, 'no-frontmatter', 'no frontmatter here'); + await seedInvalidProjectSkill(workspaceDir, 'bad-yaml', '---\nname: [unclosed\n---\nbody'); + await seedInvalidProjectSkill( + workspaceDir, + 'no-description', + '---\nname: no-description\n---\nbody', + ); + const wid = await registerWorkspace(workspaceDir); + + const { body } = await getJson<{ skills: SkillWire[] }>( + `/api/v1/workspaces/${wid}/skills`, + ); + expect(body.code).toBe(0); + const parsed = listSkillsResponseSchema.parse(body.data); + const invalid = parsed.invalid_skills.filter((s) => s.path.startsWith(workspaceDir)); + const byName = new Map( + invalid.map((s) => [s.path.split('/').at(-2) ?? '', s.reason] as const), + ); + expect([...byName.keys()].toSorted()).toEqual([ + 'bad-yaml', + 'no-description', + 'no-frontmatter', + ]); + expect(byName.get('no-frontmatter')).toContain('Missing frontmatter'); + expect(byName.get('bad-yaml')).toContain('Invalid frontmatter'); + expect(byName.get('no-description')).toContain('Missing required frontmatter field'); + expect(invalid.every((s) => s.type === 'invalid')).toBe(true); + expect( + parsed.skills.some((s) => ['no-frontmatter', 'bad-yaml', 'no-description'].includes(s.name)), + ).toBe(false); + }); }); }); From c58d5748bfcaf3fe6a04f7ac15e489ed9791c87f Mon Sep 17 00:00:00 2001 From: liukx0205 <69756503+liukx0205@users.noreply.github.com> Date: Wed, 9 Sep 2026 10:45:38 +0000 Subject: [PATCH 2/2] fix(skills): stringify non-string skill types in skip diagnostics normalizeMetadata only overrode metadata.type when it was a non-empty string, so a SKILL.md with type: 123 or type: {} kept the raw YAML value, UnsupportedSkillTypeError stored it in skillType, and the number/object leaked into the invalid_skills listing response where invalidSkillSchema requires a string, breaking strict consumers. Reject the skill with a stringified type label instead and cover the session listing path with a schema-parsing regression test. --- .../src/features/skill/catalog/parser.ts | 16 ++++++++++++- .../skill/catalog/fileSkillDiscovery.test.ts | 22 +++++++++++++++++ packages/kap-server/test/skills.test.ts | 24 +++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/packages/agent-core-v2/src/features/skill/catalog/parser.ts b/packages/agent-core-v2/src/features/skill/catalog/parser.ts index 0d3c7b8102d..63e3013e3bf 100644 --- a/packages/agent-core-v2/src/features/skill/catalog/parser.ts +++ b/packages/agent-core-v2/src/features/skill/catalog/parser.ts @@ -77,7 +77,11 @@ export function parseSkillText(options: ParseSkillTextOptions): SkillDefinition const metadata = normalizeMetadata(frontmatter); if (!isSupportedSkillType(metadata.type)) { - throw new UnsupportedSkillTypeError(metadata.type ?? String(frontmatter['type'])); + throw new UnsupportedSkillTypeError( + typeof metadata.type === 'string' + ? metadata.type + : describeUnsupportedSkillType(frontmatter['type']), + ); } const name = nonEmptyString(metadata.name); @@ -149,6 +153,16 @@ function descriptionFromBody(body: string): string { return firstLine.length > 240 ? `${firstLine.slice(0, 239)}…` : firstLine; } +function describeUnsupportedSkillType(value: unknown): string { + if (typeof value === 'string') return value; + if (typeof value === 'number' || typeof value === 'boolean') return String(value); + try { + return JSON.stringify(value) ?? String(value); + } catch { + return String(value); + } +} + function nonEmptyString(value: unknown): string | undefined { return typeof value === 'string' && value.trim() !== '' ? value.trim() : undefined; } diff --git a/packages/agent-core-v2/test/features/skill/catalog/fileSkillDiscovery.test.ts b/packages/agent-core-v2/test/features/skill/catalog/fileSkillDiscovery.test.ts index 6f0205f2e82..c60bba579a2 100644 --- a/packages/agent-core-v2/test/features/skill/catalog/fileSkillDiscovery.test.ts +++ b/packages/agent-core-v2/test/features/skill/catalog/fileSkillDiscovery.test.ts @@ -296,4 +296,26 @@ describe('FileSkillDiscovery', () => { ]); expect(warnings).toEqual([]); }); + + it('normalizes a non-string skill type to a string in the skipped entry', async () => { + await writeSkill('skills/numeric/SKILL.md', 'name: numeric\ndescription: num\ntype: 123'); + await writeSkill('skills/object/SKILL.md', 'name: object\ndescription: obj\ntype: {}'); + + const result = await discover([skillRoot('skills')]); + + expect(result.skills).toEqual([]); + expect(result.skipped).toEqual([ + { + path: join(root, 'skills/numeric/SKILL.md'), + type: '123', + reason: 'unsupported skill type "123"', + }, + { + path: join(root, 'skills/object/SKILL.md'), + type: '{}', + reason: 'unsupported skill type "{}"', + }, + ]); + expect(warnings).toEqual([]); + }); }); diff --git a/packages/kap-server/test/skills.test.ts b/packages/kap-server/test/skills.test.ts index dc0a5699b59..c622eb40e90 100644 --- a/packages/kap-server/test/skills.test.ts +++ b/packages/kap-server/test/skills.test.ts @@ -194,6 +194,30 @@ describe('server-v2 /api/v1 skills', () => { }, ]); }); + + it('normalizes a non-string skill type so the response still parses', async () => { + const workspaceDir = await makeWorkspaceDir(); + await seedInvalidProjectSkill( + workspaceDir, + 'numeric-type', + '---\nname: numeric-type\ndescription: bad type\ntype: 123\n---\nbody', + ); + const id = await createSession(workspaceDir); + + const { body } = await getJson<{ skills: SkillWire[] }>( + `/api/v1/sessions/${id}/skills`, + ); + expect(body.code).toBe(0); + const parsed = listSkillsResponseSchema.parse(body.data); + const invalid = parsed.invalid_skills.filter((s) => s.path.startsWith(workspaceDir)); + expect(invalid).toEqual([ + { + path: join(workspaceDir, '.kimi-code', 'skills', 'numeric-type', 'SKILL.md'), + type: '123', + reason: 'unsupported skill type "123"', + }, + ]); + }); }); describe('POST /api/v1/sessions/{sid}/skills/{name}:activate', () => {