Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/invalid-skill-visibility.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 15 additions & 1 deletion packages/agent-core-v2/src/features/skill/catalog/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
Expand All @@ -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');

Expand All @@ -264,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([]);
});
});
8 changes: 8 additions & 0 deletions packages/kap-server/src/protocol/rest-skill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof invalidSkillSchema>;

export const listSkillsResponseSchema = z.object({
skills: z.array(skillDescriptorSchema),
invalid_skills: z.array(invalidSkillSchema),
});
export type ListSkillsResponse = z.infer<typeof listSkillsResponseSchema>;

Expand Down
25 changes: 16 additions & 9 deletions packages/kap-server/src/routes/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
},
Expand All @@ -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));
Comment thread
liukx0205 marked this conversation as resolved.
},
);
app.get(
Expand All @@ -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',
},
Expand All @@ -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(
Expand Down Expand Up @@ -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);
}
},
);
Expand All @@ -301,7 +307,7 @@ export function registerSkillsRoutes(app: SkillsRouteHost, core: Scope): void {
async function listWorkspaceSkillsForRoot(
core: Scope,
workDir: string,
): Promise<readonly SkillDefinition[]> {
): 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);
Expand Down Expand Up @@ -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<ISessionSkillCatalog['catalog']['listSkills']>[number];
Expand Down
96 changes: 96 additions & 0 deletions packages/kap-server/test/skills.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ describe('server-v2 /api/v1 skills', () => {
);
}

async function seedInvalidProjectSkill(root: string, name: string, content: string): Promise<void> {
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<void> {
const dir = join(root, name);
await mkdir(dir, { recursive: true });
Expand Down Expand Up @@ -167,6 +173,51 @@ 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'),
},
]);
});

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', () => {
Expand Down Expand Up @@ -475,5 +526,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);
});
});
});
Loading