From ee3ba0857c33e7a84558323bf5a154f42b16aa47 Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Tue, 21 Jul 2026 09:34:20 +0700 Subject: [PATCH 1/2] fix(agent-core-v2): surface a warning when a skill fails to load A SKILL.md whose frontmatter fails to parse (e.g. a description with an unquoted colon that breaks YAML) was only logged to the internal log file and otherwise vanished from the skill list with no visible signal. Record these as skipped skills (same mechanism already used for unsupported skill types) and surface them through GET /sessions/{id}/warnings, which the TUI already shows at session start. --- .changeset/skill-load-failed-warning.md | 7 +++++ .../app/skillCatalog/fileSkillDiscovery.ts | 7 +++++ .../skillCatalog/fileSkillDiscovery.test.ts | 24 +++++++++++++++++ packages/kap-server/src/routes/sessions.ts | 26 ++++++++++++++++--- packages/kap-server/test/sessions.test.ts | 24 +++++++++++++++++ 5 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 .changeset/skill-load-failed-warning.md diff --git a/.changeset/skill-load-failed-warning.md b/.changeset/skill-load-failed-warning.md new file mode 100644 index 0000000000..0dcbf82fd6 --- /dev/null +++ b/.changeset/skill-load-failed-warning.md @@ -0,0 +1,7 @@ +--- +"@moonshot-ai/kimi-code": patch +"@moonshot-ai/agent-core-v2": patch +"@moonshot-ai/kap-server": patch +--- + +Fix a skill with a `SKILL.md` that fails to parse (e.g. invalid frontmatter) being dropped from the skill list with no signal. A session-start warning now names the skill file and the reason it was not loaded. diff --git a/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts b/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts index 5d40eca819..8de89b01d5 100644 --- a/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts +++ b/packages/agent-core-v2/src/app/skillCatalog/fileSkillDiscovery.ts @@ -183,6 +183,13 @@ async function parseAndRegister(input: { }); } else if (error instanceof SkillParseError) { input.warn?.(`Skipping invalid skill at ${input.skillMdPath}: ${error.message}`, error); + // Also record it as skipped (not just logged) so it surfaces as a + // session warning instead of vanishing silently. + input.skipped.push({ + path: input.skillMdPath, + type: 'invalid-frontmatter', + reason: error.message, + }); } else { input.warn?.(`Skipping skill at ${input.skillMdPath} due to unexpected error`, error); } diff --git a/packages/agent-core-v2/test/app/skillCatalog/fileSkillDiscovery.test.ts b/packages/agent-core-v2/test/app/skillCatalog/fileSkillDiscovery.test.ts index e82d2be20c..8bbed95ae6 100644 --- a/packages/agent-core-v2/test/app/skillCatalog/fileSkillDiscovery.test.ts +++ b/packages/agent-core-v2/test/app/skillCatalog/fileSkillDiscovery.test.ts @@ -217,6 +217,13 @@ describe('FileSkillDiscovery', () => { const result = await discover([skillRoot('skills')]); expect(result.skills).toEqual([]); + expect(result.skipped).toEqual([ + { + path: skillMdPath, + type: 'invalid-frontmatter', + reason: `Missing frontmatter in ${skillMdPath}`, + }, + ]); expect(warnings).toEqual([ { message: `Skipping invalid skill at ${skillMdPath}: Missing frontmatter in ${skillMdPath}`, @@ -225,6 +232,23 @@ describe('FileSkillDiscovery', () => { ]); }); + it('skips (rather than silently drops) a description with an unquoted colon that breaks YAML', async () => { + // Regression for #1972: a `description` containing a bare "word: word" + // (e.g. "Triggers on: ...") is invalid as a plain YAML scalar, so the + // frontmatter fails to parse. The skill used to vanish with no signal + // beyond a log line nobody sees; it must now show up in `skipped`. + await writeSkill( + 'skills/codebase-memory/SKILL.md', + 'name: codebase-memory\ndescription: Explore the codebase. Triggers on: find callers of, trace the call chain.', + ); + + const result = await discover([skillRoot('skills')]); + + expect(result.skills).toEqual([]); + expect(result.skipped).toHaveLength(1); + expect(result.skipped[0]?.type).toBe('invalid-frontmatter'); + }); + 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/routes/sessions.ts b/packages/kap-server/src/routes/sessions.ts index 1623004e09..9f034a82db 100644 --- a/packages/kap-server/src/routes/sessions.ts +++ b/packages/kap-server/src/routes/sessions.ts @@ -38,11 +38,15 @@ * `GET /sessions/{id}/warnings` surfaces session-level notices in the v1 * `{ code, message, severity }` wire shape: the `agents-md-oversized` warning * (projected from the main agent's `IAgentProfileService.getAgentsMdWarning()` - * — computed and cached when the agent binds a profile) and the - * secondary-model early-validation warning (projected from the Session-scope + * — computed and cached when the agent binds a profile), the secondary-model + * early-validation warning (projected from the Session-scope * `ISessionSecondaryModelWarningService` — computed and cached when the main - * agent is created). An unbound main agent or a valid/unset secondary model - * yields an empty list, matching v1's "no warning" case. + * agent is created), and one `skill-load-failed` entry per + * `ISessionSkillCatalog` `getSkippedByPolicy()` result whose `SKILL.md` failed + * to parse (invalid frontmatter, unsupported `type`, …), so a skill dropped + * during load is reported instead of vanishing silently. An unbound main agent, + * a valid/unset secondary model, and a clean catalog together yield an empty + * list, matching v1's "no warning" case. * * **Wire fidelity**: mirrors v1's `toProtocolSession` * (`packages/agent-core/src/services/session/session.ts`), which populates @@ -89,6 +93,7 @@ import { ISessionMetadata, ISessionLegacyService, ISessionSecondaryModelWarningService, + ISessionSkillCatalog, IEventService, IWorkspaceAliases, IWorkspaceService, @@ -1011,6 +1016,18 @@ export function registerSessionsRoutes(app: SessionRouteHost, core: Scope): void const secondaryModelWarning = session.accessor .get(ISessionSecondaryModelWarningService) .getSecondaryModelWarning(); + + // Surface any skill that got dropped during catalog load (invalid + // frontmatter, unsupported `type`, …) instead of leaving it silently + // missing from the skill list. + const skillCatalog = session.accessor.get(ISessionSkillCatalog); + await skillCatalog.ready; + const skillWarnings = skillCatalog.catalog.getSkippedByPolicy().map((skipped) => ({ + code: 'skill-load-failed', + message: `Skill at ${skipped.path} was not loaded: ${skipped.reason}`, + severity: 'warning' as const, + })); + const warnings = [ ...(agentsMdWarning === undefined ? [] @@ -1030,6 +1047,7 @@ export function registerSessionsRoutes(app: SessionRouteHost, core: Scope): void severity: 'warning' as const, }, ]), + ...skillWarnings, ]; reply.send(okEnvelope({ warnings }, req.id)); } catch (error) { diff --git a/packages/kap-server/test/sessions.test.ts b/packages/kap-server/test/sessions.test.ts index b6a2db27d9..5147f054ea 100644 --- a/packages/kap-server/test/sessions.test.ts +++ b/packages/kap-server/test/sessions.test.ts @@ -792,6 +792,30 @@ describe('server-v2 /api/v1/sessions', () => { expect(sessionWarningsResponseSchema.parse(body.data)).toEqual({ warnings: [] }); }); + it('reports a skill-load-failed warning for a SKILL.md that fails to parse', async () => { + // Regression for #1972: an unquoted colon in `description` (e.g. + // "Triggers on: ...") makes the frontmatter invalid YAML, and the skill + // used to be dropped with no signal reaching the user. + const cwd = home as string; + const skillDir = join(cwd, '.kimi-code', 'skills', 'broken'); + await mkdir(skillDir, { recursive: true }); + await writeFile( + join(skillDir, 'SKILL.md'), + '---\nname: broken\ndescription: Explore the codebase. Triggers on: find callers of.\n---\n\nbody\n', + ); + + const created = await postJson('/api/v1/sessions', { metadata: { cwd } }); + const { status, body } = await getJson<{ + warnings: { code: string; message: string; severity: string }[]; + }>(`/api/v1/sessions/${created.body.data.id}/warnings`); + + expect(status).toBe(200); + expect(body.code).toBe(0); + const skillWarning = body.data.warnings.find((w) => w.code === 'skill-load-failed'); + expect(skillWarning?.message).toContain(join(skillDir, 'SKILL.md')); + expect(sessionWarningsResponseSchema.parse(body.data).warnings).toContainEqual(skillWarning); + }); + it('returns 40401 for warnings of a missing session', async () => { const { body } = await getJson('/api/v1/sessions/sess_missing_warnings/warnings'); expect(body.code).toBe(40401); From d224948189bfc5b2a293a983a0111f49c5baa8ec Mon Sep 17 00:00:00 2001 From: arimu1 <19286898+arimu1@users.noreply.github.com> Date: Tue, 21 Jul 2026 10:06:06 +0700 Subject: [PATCH 2/2] fix(agent-core): surface skill-load-failed warnings on the v1 engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v2/kap-server fix for #1972 never reached the interactive TUI: it calls Session.getSessionWarnings() through an in-process KimiCore bound to agent-core (v1), not kap-server. Mirror the same fix there — record a SkillParseError as a skipped skill (byte-identical to the existing UnsupportedSkillTypeError handling) and surface it from getSessionWarnings() via the registry's (previously unused) getSkippedByPolicy(). --- .changeset/skill-load-failed-warning.md | 3 +- packages/agent-core/src/session/index.ts | 11 ++++++++ packages/agent-core/src/skill/scanner.ts | 7 +++++ .../test/harness/skill-session.test.ts | 23 +++++++++++++++ .../agent-core/test/skill/scanner.test.ts | 28 +++++++++++++++++++ 5 files changed, 71 insertions(+), 1 deletion(-) diff --git a/.changeset/skill-load-failed-warning.md b/.changeset/skill-load-failed-warning.md index 0dcbf82fd6..a80ffbb938 100644 --- a/.changeset/skill-load-failed-warning.md +++ b/.changeset/skill-load-failed-warning.md @@ -1,7 +1,8 @@ --- "@moonshot-ai/kimi-code": patch +"@moonshot-ai/agent-core": patch "@moonshot-ai/agent-core-v2": patch "@moonshot-ai/kap-server": patch --- -Fix a skill with a `SKILL.md` that fails to parse (e.g. invalid frontmatter) being dropped from the skill list with no signal. A session-start warning now names the skill file and the reason it was not loaded. +Fix a skill with a `SKILL.md` that fails to parse (e.g. invalid frontmatter) being dropped from the skill list with no signal. A session-start warning now names the skill file and the reason it was not loaded, in both the CLI/TUI engine and `kimi web`/print mode. diff --git a/packages/agent-core/src/session/index.ts b/packages/agent-core/src/session/index.ts index 50762884f8..9f0bedb20f 100644 --- a/packages/agent-core/src/session/index.ts +++ b/packages/agent-core/src/session/index.ts @@ -802,6 +802,17 @@ export class Session { }); } warnings.push(...this.computeSecondaryModelWarnings()); + // Surface any skill that got dropped during catalog load (invalid + // frontmatter, unsupported `type`, …) instead of leaving it silently + // missing from the skill list. + await this.skillsReady; + for (const skipped of this.skills.getSkippedByPolicy()) { + warnings.push({ + code: 'skill-load-failed', + message: `Skill at ${skipped.path} was not loaded: ${skipped.reason}`, + severity: 'warning', + }); + } return warnings; } diff --git a/packages/agent-core/src/skill/scanner.ts b/packages/agent-core/src/skill/scanner.ts index 6c1ddb462c..53481c5e1c 100644 --- a/packages/agent-core/src/skill/scanner.ts +++ b/packages/agent-core/src/skill/scanner.ts @@ -407,6 +407,13 @@ async function parseAndRegister(input: { }); } else if (error instanceof SkillParseError) { input.warn(`Skipping invalid skill at ${input.skillMdPath}: ${error.message}`, error); + // Also record it as skipped (not just logged) so it surfaces as a + // session warning instead of vanishing silently. + input.skip({ + path: input.skillMdPath, + type: 'invalid-frontmatter', + reason: error.message, + }); } else { input.warn(`Skipping skill at ${input.skillMdPath} due to unexpected error`, error); } diff --git a/packages/agent-core/test/harness/skill-session.test.ts b/packages/agent-core/test/harness/skill-session.test.ts index 987380961c..e12eb39c22 100644 --- a/packages/agent-core/test/harness/skill-session.test.ts +++ b/packages/agent-core/test/harness/skill-session.test.ts @@ -611,6 +611,29 @@ describe('HarnessAPI session skills', () => { }); }); + it('reports a skill-load-failed session warning for a SKILL.md that fails to parse', async () => { + // Regression for #1972: an unquoted colon in `description` (e.g. + // "Triggers on: ...") makes the frontmatter invalid YAML, and the skill + // used to be dropped with no signal reaching the TUI, which calls this + // same `getSessionWarnings` RPC at session start. + await writeSkill('broken', [ + '---', + 'name: broken', + 'description: Explore the codebase. Triggers on: find callers of.', + '---', + '', + 'Body.', + ]); + const { rpc } = await createTestRpc(); + const created = await rpc.createSession({ id: 'ses_skill_warning', workDir }); + + const warnings = await rpc.getSessionWarnings({ sessionId: created.id }); + + const skillWarning = warnings.find((warning) => warning.code === 'skill-load-failed'); + expect(skillWarning?.message).toContain('broken/SKILL.md'); + expect(skillWarning?.severity).toBe('warning'); + }); + it('rejects missing and non-inline skills with structured errors', async () => { const { core, rpc } = await createTestRpc(); const created = await rpc.createSession({ id: 'ses_skill_errors', workDir }); diff --git a/packages/agent-core/test/skill/scanner.test.ts b/packages/agent-core/test/skill/scanner.test.ts index 38377338ea..21bc2bce66 100644 --- a/packages/agent-core/test/skill/scanner.test.ts +++ b/packages/agent-core/test/skill/scanner.test.ts @@ -196,6 +196,34 @@ describe('skill discovery', () => { expect(warnings.some((message) => message.includes('"name"'))).toBe(true); expect(warnings.some((message) => message.includes('"description"'))).toBe(true); }); + + it('reports a skill with unparseable frontmatter as skipped, not just warned', async () => { + // Regression for #1972: a `description` containing a bare "word: word" + // (e.g. "Triggers on: ...") is invalid as a plain YAML scalar, so the + // frontmatter fails to parse. The skill used to vanish with only a log + // line nobody sees; it must now show up in the skipped list too. + const { repoDir } = await makeWorkspace(); + const projectRoot = path.join(repoDir, '.kimi-code', 'skills'); + await writeSkill(projectRoot, path.join('codebase-memory', 'SKILL.md'), [ + '---', + 'name: codebase-memory', + 'description: Explore the codebase. Triggers on: find callers of.', + '---', + '', + 'Body.', + ]); + + const skipped: { path: string; type: string; reason: string }[] = []; + const skills = await discoverSkills({ + roots: [{ path: projectRoot, source: 'project' }], + onSkippedByPolicy: (skill) => skipped.push(skill), + }); + + expect(skills).toEqual([]); + expect(skipped).toHaveLength(1); + expect(skipped[0]?.type).toBe('invalid-frontmatter'); + expect(skipped[0]?.path).toContain('codebase-memory/SKILL.md'); + }); }); describe('discoverSkills shape and ordering', () => {