-
Notifications
You must be signed in to change notification settings - Fork 124
fix(skill): wrap slash-activated skill content in system-reminder tags #135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b80b9a8
b70a774
cc2ed2c
bc17dda
900026c
93634b9
2b92b66
12caa72
355e73f
4a311c9
46c00f3
43f264c
e47f52f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| "@moonshot-ai/agent-core": patch | ||
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Fix slash-activated skills not being recognized by the model due to missing system reminder wrapper. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import type { ContentPart } from '@moonshot-ai/kosong'; | |
| import type { Agent } from '..'; | ||
| import { ErrorCodes, KimiError } from '#/errors'; | ||
| import { isUserActivatableSkillType, type SkillRegistry } from '../../skill'; | ||
| import { escapeXml } from '#/utils/xml-escape'; | ||
| import type { SkillActivationOrigin } from '../context'; | ||
|
|
||
| export class SkillManager { | ||
|
|
@@ -23,6 +24,21 @@ export class SkillManager { | |
| throw new KimiError(ErrorCodes.SKILL_TYPE_UNSUPPORTED, `Skill "${skill.name}" cannot be activated by the user`); | ||
| } | ||
|
|
||
| const skillContent = this.registry.renderSkillPrompt(skill, input.args ?? ''); | ||
| const args = input.args; | ||
| const argsAttr = args ? ` args="${escapeXml(args)}"` : ''; | ||
| const wrapped = [ | ||
| { | ||
| type: 'text' as const, | ||
| text: | ||
| `<system-reminder>\n` + | ||
| `<kimi-skill-loaded name="${escapeXml(skill.name)}"${argsAttr}>\n` + | ||
| `${skillContent}\n` + | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a slash-activated skill body contains Useful? React with 👍 / 👎. |
||
| `</kimi-skill-loaded>\n` + | ||
| `</system-reminder>`, | ||
| }, | ||
| ]; | ||
|
|
||
| this.recordActivation( | ||
| { | ||
| kind: 'skill_activation', | ||
|
|
@@ -34,12 +50,7 @@ export class SkillManager { | |
| skillSource: skill.source, | ||
| skillArgs: input.args, | ||
| }, | ||
| [ | ||
| { | ||
| type: 'text', | ||
| text: this.registry.renderSkillPrompt(skill, input.args ?? ''), | ||
| }, | ||
| ], | ||
| wrapped, | ||
| ); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| /** Escape XML content — escapes both tag and attribute boundary chars (& < > ") */ | ||
| export function escapeXml(input: string): string { | ||
| return input | ||
| .replaceAll('&', '&') | ||
| .replaceAll('<', '<') | ||
| .replaceAll('>', '>') | ||
| .replaceAll('"', '"'); | ||
| } | ||
|
|
||
| /** Escape XML attribute value — only escapes attribute boundary chars (& "), not tag chars */ | ||
| export function escapeXmlAttr(input: string): string { | ||
| return input.replaceAll('&', '&').replaceAll('"', '"'); | ||
| } | ||
|
|
||
| /** Escape tag delimiters only — prevents XML tag injection without corrupting Markdown (& " stay literal) */ | ||
| export function escapeXmlTags(input: string): string { | ||
| return input.replaceAll('<', '<').replaceAll('>', '>'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a slash command includes args and the skill body does not consume placeholders,
renderSkillPrompt()appendsARGUMENTS: ${rawArgs}; this line then places that raw user-controlled text inside the new<system-reminder>wrapper. For inputs containing XML delimiters such as</kimi-skill-loaded></system-reminder>(or directive-like text), the args can terminate or corrupt the wrapper and be treated as authoritative reminder content rather than data, defeating the boundary this change is trying to add. Consider escaping/sandboxing rendered arguments or keeping them outside the reminder.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it ,i will fix it