mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-07-09 17:29:12 +00:00
fix: surface skill directory in the loaded-skill context block (#785)
This commit is contained in:
parent
2746c71c47
commit
4578f05f44
9 changed files with 141 additions and 12 deletions
6
.changeset/skill-dir-in-context.md
Normal file
6
.changeset/skill-dir-in-context.md
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
"@moonshot-ai/agent-core": patch
|
||||
"@moonshot-ai/kimi-code": patch
|
||||
---
|
||||
|
||||
Include the skill's directory on the loaded-skill context block so the agent can locate a skill's bundled resources (scripts, templates) after it is invoked.
|
||||
|
|
@ -37,6 +37,7 @@ export class SkillManager {
|
|||
skillArgs,
|
||||
skillContent,
|
||||
skillSource: skill.source,
|
||||
skillDir: skill.dir,
|
||||
}),
|
||||
},
|
||||
];
|
||||
|
|
|
|||
|
|
@ -8,6 +8,14 @@ export interface RenderSkillPromptInput {
|
|||
readonly skillArgs: string;
|
||||
readonly skillContent: string;
|
||||
readonly skillSource?: SkillSource | undefined;
|
||||
/**
|
||||
* Absolute directory containing the skill's SKILL.md and any bundled
|
||||
* resources (scripts, templates, data files). Surfaced on the loaded
|
||||
* block so the agent can locate those resources with relative paths —
|
||||
* without it, a skill that ships helper scripts is unusable unless the
|
||||
* author manually embeds `${KIMI_SKILL_DIR}` in the body.
|
||||
*/
|
||||
readonly skillDir?: string | undefined;
|
||||
}
|
||||
|
||||
interface RenderSkillLoadedBlockInput extends RenderSkillPromptInput {
|
||||
|
|
@ -47,6 +55,7 @@ function renderSkillAttributes(input: RenderSkillLoadedBlockInput): string {
|
|||
['name', input.skillName],
|
||||
['trigger', input.trigger],
|
||||
['source', input.skillSource],
|
||||
['dir', input.skillDir],
|
||||
['args', input.skillArgs],
|
||||
];
|
||||
|
||||
|
|
|
|||
|
|
@ -141,6 +141,7 @@ export class SkillTool implements BuiltinTool<SkillToolInput> {
|
|||
skillArgs,
|
||||
skillContent,
|
||||
skillSource: skill.source,
|
||||
skillDir: skill.dir,
|
||||
trigger: promptTrigger,
|
||||
}),
|
||||
},
|
||||
|
|
|
|||
55
packages/agent-core/test/agent/skill-prompt.test.ts
Normal file
55
packages/agent-core/test/agent/skill-prompt.test.ts
Normal file
|
|
@ -0,0 +1,55 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import {
|
||||
renderModelToolSkillPrompt,
|
||||
renderUserSlashSkillPrompt,
|
||||
} from '../../src/agent/skill/prompt';
|
||||
|
||||
/**
|
||||
* Regression coverage for the skill directory being surfaced on the
|
||||
* `<kimi-skill-loaded>` block. Without `dir`, an agent that loads a skill
|
||||
* cannot locate the skill's bundled resources (scripts, templates) by
|
||||
* relative path — the bug this guards against.
|
||||
*/
|
||||
describe('renderSkillLoadedBlock skill directory', () => {
|
||||
const base = {
|
||||
skillName: 'review',
|
||||
skillArgs: '',
|
||||
skillContent: 'body',
|
||||
skillSource: 'user' as const,
|
||||
skillDir: '/home/user/.kimi-code/skills/review',
|
||||
};
|
||||
|
||||
it('includes the skill directory for model-tool activations', () => {
|
||||
const text = renderModelToolSkillPrompt({ ...base, trigger: 'model-tool' });
|
||||
expect(text).toContain('dir="/home/user/.kimi-code/skills/review"');
|
||||
});
|
||||
|
||||
it('includes the skill directory for nested-skill activations', () => {
|
||||
const text = renderModelToolSkillPrompt({ ...base, trigger: 'nested-skill' });
|
||||
expect(text).toContain('dir="/home/user/.kimi-code/skills/review"');
|
||||
});
|
||||
|
||||
it('includes the skill directory for user-slash activations', () => {
|
||||
const text = renderUserSlashSkillPrompt(base);
|
||||
expect(text).toContain('dir="/home/user/.kimi-code/skills/review"');
|
||||
});
|
||||
|
||||
it('XML-escapes the skill directory', () => {
|
||||
const text = renderUserSlashSkillPrompt({
|
||||
...base,
|
||||
skillDir: '/skills/a&b/"weird"/<dir>',
|
||||
});
|
||||
expect(text).toContain('dir="/skills/a&b/"weird"/<dir>"');
|
||||
expect(text).not.toContain('dir="/skills/a&b/"weird"/<dir>"');
|
||||
});
|
||||
|
||||
it('omits the dir attribute when no directory is supplied', () => {
|
||||
const { skillDir: _omit, ...withoutDir } = base;
|
||||
const text = renderUserSlashSkillPrompt(withoutDir);
|
||||
expect(text).not.toContain('dir=');
|
||||
// Other attributes still render so the block is well-formed.
|
||||
expect(text).toContain('name="review"');
|
||||
expect(text).toContain('source="user"');
|
||||
});
|
||||
});
|
||||
|
|
@ -177,7 +177,7 @@ describe('ToolManager SkillTool registration', () => {
|
|||
text: [
|
||||
'Skill tool loaded instructions for this request. Follow them.',
|
||||
'',
|
||||
'<kimi-skill-loaded name="review" trigger="model-tool" source="user" args="">',
|
||||
'<kimi-skill-loaded name="review" trigger="model-tool" source="user" dir="/skills/review" args="">',
|
||||
'body of review',
|
||||
'</kimi-skill-loaded>',
|
||||
].join('\n'),
|
||||
|
|
|
|||
|
|
@ -193,10 +193,11 @@ describe('HarnessAPI session skills', () => {
|
|||
const records = await readMainWire(created.sessionDir);
|
||||
const prompt = records.find((record) => record['type'] === 'turn.prompt');
|
||||
const userMessage = records.find((record) => record['type'] === 'context.append_message');
|
||||
const skillDir = await realpath(join(workDir, '.kimi-code', 'skills', 'phase-one-review'));
|
||||
const expectedPrompt = [
|
||||
'User activated the skill "phase-one-review". Follow the loaded skill instructions.',
|
||||
'',
|
||||
'<kimi-skill-loaded name="phase-one-review" trigger="user-slash" source="project" args="src/app.ts">',
|
||||
`<kimi-skill-loaded name="phase-one-review" trigger="user-slash" source="project" dir="${skillDir}" args="src/app.ts">`,
|
||||
'Review the requested file.',
|
||||
'',
|
||||
'ARGUMENTS: src/app.ts',
|
||||
|
|
@ -286,7 +287,7 @@ describe('HarnessAPI session skills', () => {
|
|||
const expectedPrompt = [
|
||||
'User activated the skill "templated-review". Follow the loaded skill instructions.',
|
||||
'',
|
||||
'<kimi-skill-loaded name="templated-review" trigger="user-slash" source="project" args=""src/app.ts" careful">',
|
||||
`<kimi-skill-loaded name="templated-review" trigger="user-slash" source="project" dir="${skillDir}" args=""src/app.ts" careful">`,
|
||||
'Target: src/app.ts',
|
||||
'Mode: careful',
|
||||
'Raw: "src/app.ts" careful',
|
||||
|
|
@ -329,9 +330,10 @@ describe('HarnessAPI session skills', () => {
|
|||
const prompt = records.find((record) => record['type'] === 'turn.prompt');
|
||||
const text = (prompt as { input?: Array<{ text?: string }> } | undefined)?.input?.[0]?.text;
|
||||
|
||||
const skillDir = await realpath(join(workDir, '.kimi-code', 'skills', 'brainstorm'));
|
||||
expect(text).toContain('User activated the skill "brainstorm". Follow the loaded skill instructions.');
|
||||
expect(text).toContain(
|
||||
'<kimi-skill-loaded name="brainstorm" trigger="user-slash" source="project" args="">',
|
||||
`<kimi-skill-loaded name="brainstorm" trigger="user-slash" source="project" dir="${skillDir}" args="">`,
|
||||
);
|
||||
expect(text).toContain('Ask one clarifying question before proposing designs.');
|
||||
expect(text).not.toContain('<system-reminder>');
|
||||
|
|
@ -432,6 +434,7 @@ describe('HarnessAPI session skills', () => {
|
|||
const resumed = await second.rpc.resumeSession({ sessionId: created.id });
|
||||
|
||||
expect(second.events.some((event) => event.type === 'skill.activated')).toBe(false);
|
||||
const skillDir = await realpath(join(workDir, '.kimi-code', 'skills', 'phase-one-review'));
|
||||
const context = await second.rpc.getContext({ sessionId: created.id, agentId: 'main' });
|
||||
expect(context.history).toMatchObject([
|
||||
{
|
||||
|
|
@ -442,7 +445,7 @@ describe('HarnessAPI session skills', () => {
|
|||
text: [
|
||||
'User activated the skill "phase-one-review". Follow the loaded skill instructions.',
|
||||
'',
|
||||
'<kimi-skill-loaded name="phase-one-review" trigger="user-slash" source="project" args="src/app.ts">',
|
||||
`<kimi-skill-loaded name="phase-one-review" trigger="user-slash" source="project" dir="${skillDir}" args="src/app.ts">`,
|
||||
'Review the requested file.',
|
||||
'',
|
||||
'ARGUMENTS: src/app.ts',
|
||||
|
|
@ -479,6 +482,59 @@ describe('HarnessAPI session skills', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('keeps the skill directory in the resumed conversation context so bundled resources stay locatable', async () => {
|
||||
// A skill that ships a helper script but does NOT embed ${KIMI_SKILL_DIR}
|
||||
// in its body. The only way the agent can learn where the script lives is
|
||||
// the `dir` attribute on the loaded block — and it must survive a resume.
|
||||
await writeSkill('bundled-tool', [
|
||||
'---',
|
||||
'name: bundled-tool',
|
||||
'description: A skill with a bundled script',
|
||||
'---',
|
||||
'',
|
||||
'Run the bundled helper script to do the work.',
|
||||
]);
|
||||
const scriptDir = join(workDir, '.kimi-code', 'skills', 'bundled-tool', 'scripts');
|
||||
await mkdir(scriptDir, { recursive: true });
|
||||
await writeFile(join(scriptDir, 'run.sh'), '#!/bin/sh\necho hi\n');
|
||||
|
||||
const first = await createTestRpc();
|
||||
const created = await first.rpc.createSession({ id: 'ses_skill_resource_resume', workDir });
|
||||
await first.rpc.activateSkill({
|
||||
sessionId: created.id,
|
||||
agentId: 'main',
|
||||
name: 'bundled-tool',
|
||||
});
|
||||
await waitForEvent(first.events, (event) => event.type === 'skill.activated');
|
||||
await first.core.sessions.get(created.id)?.flushMetadata();
|
||||
|
||||
// Resume in a completely fresh runtime — nothing in memory, the context is
|
||||
// rebuilt from disk exactly as the model would see it on the next turn.
|
||||
const second = await createTestRpc();
|
||||
await second.rpc.resumeSession({ sessionId: created.id });
|
||||
const context = await second.rpc.getContext({ sessionId: created.id, agentId: 'main' });
|
||||
|
||||
const skillDir = await realpath(join(workDir, '.kimi-code', 'skills', 'bundled-tool'));
|
||||
const skillMessage = context.history.find(
|
||||
(entry) =>
|
||||
entry.origin?.kind === 'skill_activation' &&
|
||||
(entry.origin as { skillName?: string }).skillName === 'bundled-tool',
|
||||
);
|
||||
expect(skillMessage).toBeDefined();
|
||||
const text = (skillMessage?.content?.[0] as { text?: string } | undefined)?.text ?? '';
|
||||
|
||||
// The directory is present in the resumed context...
|
||||
expect(text).toContain(`dir="${skillDir}"`);
|
||||
// ...and it is the directory that actually holds the bundled script, so an
|
||||
// agent reading the context can resolve the resource by relative path.
|
||||
expect(join(skillDir, 'scripts', 'run.sh')).toBe(
|
||||
await realpath(join(scriptDir, 'run.sh')),
|
||||
);
|
||||
// Guard the regression: the path is surfaced by the wrapper, not because
|
||||
// the skill body happened to mention it.
|
||||
expect(text).toContain('Run the bundled helper script to do the work.');
|
||||
});
|
||||
|
||||
it('registers builtin mcp-config skill, hides it from the model, and activates it via slash', async () => {
|
||||
const { core, events, rpc } = await createTestRpc();
|
||||
const created = await rpc.createSession({ id: 'ses_skill_builtin', workDir });
|
||||
|
|
|
|||
|
|
@ -147,7 +147,7 @@ describe('SkillTool execution', () => {
|
|||
expect(methods.recordUserMessage).toHaveBeenCalledTimes(1);
|
||||
expect(methods.recordUserMessage.mock.calls[0]?.[0][0]?.text).toBe(
|
||||
'Skill tool loaded instructions for this request. Follow them.\n\n' +
|
||||
'<kimi-skill-loaded name="commit" trigger="model-tool" source="user" args="message text">\nbody of commit\n\nARGUMENTS: message text\n</kimi-skill-loaded>',
|
||||
'<kimi-skill-loaded name="commit" trigger="model-tool" source="user" dir="/skills/commit" args="message text">\nbody of commit\n\nARGUMENTS: message text\n</kimi-skill-loaded>',
|
||||
);
|
||||
expect(methods.recordUserMessage.mock.calls[0]?.[0][0]?.text).not.toContain(
|
||||
'<system-reminder>',
|
||||
|
|
@ -174,7 +174,7 @@ describe('SkillTool execution', () => {
|
|||
|
||||
expect(methods.recordUserMessage.mock.calls[0]?.[0][0]?.text).toBe(
|
||||
'Skill tool loaded instructions for this request. Follow them.\n\n' +
|
||||
'<kimi-skill-loaded name="brainstorming" trigger="model-tool" source="extra" args="">\n' +
|
||||
'<kimi-skill-loaded name="brainstorming" trigger="model-tool" source="extra" dir="/skills/brainstorming" args="">\n' +
|
||||
'<kimi-plugin-instructions plugin="superpowers">\n' +
|
||||
'Use AskUserQuestion for clarifying questions.\n' +
|
||||
'</kimi-plugin-instructions>\n\nbrainstorm body\n' +
|
||||
|
|
@ -199,7 +199,7 @@ describe('SkillTool execution', () => {
|
|||
|
||||
expect(methods.recordUserMessage.mock.calls[0]?.[0][0]?.text).toBe(
|
||||
'Skill tool loaded instructions for this request. Follow them.\n\n' +
|
||||
'<kimi-skill-loaded name="commit" trigger="model-tool" source="user" args="-m "fix login"">\nFlag: -m\nCommit message: fix login\nRaw: -m "fix login"\n</kimi-skill-loaded>',
|
||||
'<kimi-skill-loaded name="commit" trigger="model-tool" source="user" dir="/skills/commit" args="-m "fix login"">\nFlag: -m\nCommit message: fix login\nRaw: -m "fix login"\n</kimi-skill-loaded>',
|
||||
);
|
||||
expect(methods.recordUserMessage.mock.calls[0]?.[0][0]?.text).not.toContain('ARGUMENTS:');
|
||||
});
|
||||
|
|
@ -217,7 +217,7 @@ describe('SkillTool execution', () => {
|
|||
|
||||
expect(methods.recordUserMessage.mock.calls[0]?.[0][0]?.text).toBe(
|
||||
'Skill tool loaded instructions for this request. Follow them.\n\n' +
|
||||
'<kimi-skill-loaded name="session-aware" trigger="model-tool" source="user" args="">\nSession: ses_model_skill\n</kimi-skill-loaded>',
|
||||
'<kimi-skill-loaded name="session-aware" trigger="model-tool" source="user" dir="/skills/session-aware" args="">\nSession: ses_model_skill\n</kimi-skill-loaded>',
|
||||
);
|
||||
});
|
||||
|
||||
|
|
@ -251,7 +251,7 @@ describe('SkillTool execution', () => {
|
|||
|
||||
expect(methods.recordUserMessage.mock.calls[0]?.[0][0]?.text).toBe(
|
||||
'Skill tool loaded instructions for this request. Follow them.\n\n' +
|
||||
'<kimi-skill-loaded name="a&b" trigger="model-tool" source="user" args="<raw "value">">\nbody of a&b\n\nARGUMENTS: <raw "value">\n</kimi-skill-loaded>',
|
||||
'<kimi-skill-loaded name="a&b" trigger="model-tool" source="user" dir="/skills/a&b" args="<raw "value">">\nbody of a&b\n\nARGUMENTS: <raw "value">\n</kimi-skill-loaded>',
|
||||
);
|
||||
expect(methods.recordSkillActivation).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { mkdir, readFile, writeFile } from 'node:fs/promises';
|
||||
import { mkdir, readFile, realpath, writeFile } from 'node:fs/promises';
|
||||
import { join } from 'node:path';
|
||||
|
||||
import type * as KosongModule from '@moonshot-ai/kosong';
|
||||
|
|
@ -172,6 +172,7 @@ describe('Session skills', () => {
|
|||
expect(state['isCustomTitle']).toBe(false);
|
||||
expect(state['lastPrompt']).toBe('/review src/app.ts');
|
||||
|
||||
const skillDir = await realpath(join(workDir, '.kimi-code', 'skills', 'review'));
|
||||
await expect(
|
||||
waitForAgentWireEvent(
|
||||
homeDir,
|
||||
|
|
@ -187,7 +188,7 @@ describe('Session skills', () => {
|
|||
text: [
|
||||
'User activated the skill "review". Follow the loaded skill instructions.',
|
||||
'',
|
||||
'<kimi-skill-loaded name="review" trigger="user-slash" source="project" args="src/app.ts">',
|
||||
`<kimi-skill-loaded name="review" trigger="user-slash" source="project" dir="${skillDir}" args="src/app.ts">`,
|
||||
'Review the requested file.',
|
||||
'',
|
||||
'ARGUMENTS: src/app.ts',
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue