Skip to content

Commit 13e7d1a

Browse files
authored
🎨 Support loading bundled Skill resource files (#19031)
* ✨ Support bundled Skill resources * 🎨 Clarify Skill resource variable handling * 🐛 Reject non-UTF-8 skill resources
1 parent 7aa4a48 commit 13e7d1a

5 files changed

Lines changed: 571 additions & 18 deletions

File tree

kernel/mcp/server_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,14 @@ import (
2222
"io"
2323
"net/http"
2424
"net/http/httptest"
25+
"os"
26+
"path/filepath"
2527
"strings"
2628
"testing"
2729

2830
mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp"
2931
"github.com/siyuan-note/siyuan/kernel/mcp/tools"
32+
"github.com/siyuan-note/siyuan/kernel/util"
3033
)
3134

3235
func newTestHTTPServer(t *testing.T) (*mcpsdk.Server, *httptest.Server) {
@@ -392,6 +395,44 @@ func TestToolInputAndOutputValidation(t *testing.T) {
392395
}
393396
}
394397

398+
func TestSkillResourceContentSurvivesMCPConversion(t *testing.T) {
399+
originalDataDir, originalHomeDir := util.DataDir, util.HomeDir
400+
root := t.TempDir()
401+
util.DataDir = filepath.Join(root, "workspace", "data")
402+
util.HomeDir = filepath.Join(root, "home")
403+
t.Cleanup(func() {
404+
util.DataDir, util.HomeDir = originalDataDir, originalHomeDir
405+
})
406+
407+
skillDir := filepath.Join(util.SkillsDir(), "transport-skill")
408+
if err := os.MkdirAll(filepath.Join(skillDir, "references"), 0755); err != nil {
409+
t.Fatal(err)
410+
}
411+
if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"),
412+
[]byte("---\nname: transport-skill\ndescription: transport test\n---\nbody"), 0644); err != nil {
413+
t.Fatal(err)
414+
}
415+
if err := os.WriteFile(filepath.Join(skillDir, "references", "valid.txt"), []byte("中文内容"), 0644); err != nil {
416+
t.Fatal(err)
417+
}
418+
419+
result, err := tools.SkillTool.Handler(map[string]any{
420+
"action": "load",
421+
"name": "transport-skill/references/valid.txt",
422+
})
423+
if err != nil || result.IsError || len(result.Content) != 1 {
424+
t.Fatalf("unexpected Skill resource result: %#v, %v", result, err)
425+
}
426+
converted, err := convertContentItem(result.Content[0])
427+
if err != nil {
428+
t.Fatal(err)
429+
}
430+
text, ok := converted.(*mcpsdk.TextContent)
431+
if !ok || text.Text != "<skill_resource skill=\"transport-skill\" path=\"references/valid.txt\">\n\n中文内容\n\n</skill_resource>" {
432+
t.Fatalf("Skill resource changed during MCP conversion: %#v", converted)
433+
}
434+
}
435+
395436
func TestNonTextToolContentIsPreserved(t *testing.T) {
396437
server, httpServer := newTestHTTPServer(t)
397438
var imageContent tools.ContentItem

kernel/mcp/tools/skill.go

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package tools
1919
import (
2020
"fmt"
2121
"html"
22+
"path/filepath"
2223
"strings"
2324

2425
"github.com/siyuan-note/siyuan/kernel/model"
@@ -27,12 +28,15 @@ import (
2728

2829
var SkillTool = &Tool{
2930
Name: "skill",
30-
Description: "Skill operations: load(name), save(name, content), install(url), remove(name), rename(name, new_name), list().\n\n" + skillListDesc(),
31+
Description: "Skill operations: load(name) loads Skill instructions; load(name/resource-path) loads a bundled text resource; save(name, content), install(url), remove(name), rename(name, new_name), list().\n\n" + skillListDesc(),
3132
InputSchema: ToolSchema{
3233
Type: "object",
3334
Properties: map[string]Property{
34-
"action": {Type: "string", Description: "Operation", Enum: []string{"load", "save", "install", "remove", "rename", "list"}},
35-
"name": {Type: "string", Description: "Skill name (directory name)"},
35+
"action": {Type: "string", Description: "Operation", Enum: []string{"load", "save", "install", "remove", "rename", "list"}},
36+
"name": {
37+
Type: "string",
38+
Description: "Skill name. For load, use <skill-name> for instructions or <skill-name>/<relative-resource-path> for a bundled text resource",
39+
},
3640
"content": {Type: "string", Description: "SKILL.md full content with YAML frontmatter (for save)"},
3741
"url": {Type: "string", Description: "Skill source for install: 'owner/repo' shorthand (e.g. Tencent/WeChatReading), a full GitHub URL, a raw SKILL.md URL, or a release zip URL"},
3842
"new_name": {Type: "string", Description: "New skill name (for rename)"},
@@ -87,23 +91,78 @@ func skillLoad(args map[string]any) (CallToolResult, error) {
8791
}, nil
8892
}
8993

90-
content := util.LoadSkillContent(name, model.EnabledUserSkills())
91-
if content == "" {
94+
loaded, err := util.LoadSkill(name, model.EnabledUserSkills())
95+
if err != nil {
9296
return CallToolResult{
93-
Content: []ContentItem{{Type: "text", Text: fmt.Sprintf("skill not found: %s", name)}},
97+
Content: []ContentItem{{Type: "text", Text: err.Error()}},
9498
IsError: true,
9599
}, nil
96100
}
97101

98-
// 变量(非敏感)在技能正文注入对话时解析,让 LLM 看到实际值;密钥不进上下文。
99-
content = model.Conf.Variables.Resolve(content)
100-
101-
result := "<skill_content name=\"" + html.EscapeString(name) + "\">\n\n" + content + "\n\n</skill_content>"
102+
var result string
103+
if loaded.ResourcePath == "" {
104+
// Variables.Resolve 还会匹配 $NAME 和 ${NAME}。
105+
// 资源可能包含脚本或模板,因此只解析技能正文,避免误改资源内容。
106+
loaded.Content = model.Conf.Variables.Resolve(loaded.Content)
107+
result = formatSkillContent(loaded)
108+
} else {
109+
result = formatSkillResource(loaded)
110+
}
102111
return CallToolResult{
103112
Content: []ContentItem{{Type: "text", Text: result}},
104113
}, nil
105114
}
106115

116+
func formatSkillContent(loaded *util.SkillLoadResult) string {
117+
var sb strings.Builder
118+
sb.WriteString(`<skill_content name="`)
119+
sb.WriteString(html.EscapeString(loaded.Name))
120+
sb.WriteString("\">\n\n")
121+
sb.WriteString(loaded.Content)
122+
123+
if len(loaded.Resources) > 0 || loaded.ResourcesTruncated {
124+
sb.WriteString("\n\n<skill_resources")
125+
if loaded.ResourcesTruncated {
126+
sb.WriteString(` truncated="true"`)
127+
}
128+
sb.WriteString(">\n")
129+
for _, resource := range loaded.Resources {
130+
sb.WriteString(" <file>")
131+
sb.WriteString(html.EscapeString(resource))
132+
sb.WriteString("</file>\n")
133+
}
134+
sb.WriteString("</skill_resources>")
135+
}
136+
137+
if location := workspaceSkillLocation(loaded.SkillDir); location != "" {
138+
sb.WriteString("\n\n<skill_location path=\"")
139+
sb.WriteString(html.EscapeString(location))
140+
sb.WriteString("\">\n")
141+
sb.WriteString(" For this skill directory, use the `file` tool's read-only actions: `read`, `list`, `grep`, and `find`.\n")
142+
sb.WriteString("</skill_location>")
143+
}
144+
145+
sb.WriteString("\n\n</skill_content>")
146+
return sb.String()
147+
}
148+
149+
func formatSkillResource(loaded *util.SkillLoadResult) string {
150+
return `<skill_resource skill="` + html.EscapeString(loaded.Name) + `" path="` +
151+
html.EscapeString(loaded.ResourcePath) + "\">\n\n" + loaded.Content + "\n\n</skill_resource>"
152+
}
153+
154+
func workspaceSkillLocation(skillDir string) string {
155+
location, err := filepath.Rel(util.WorkspaceDir, skillDir)
156+
if err != nil {
157+
return ""
158+
}
159+
location = filepath.ToSlash(location)
160+
if _, err = resolvePath(location); err != nil {
161+
return ""
162+
}
163+
return location
164+
}
165+
107166
func skillSave(args map[string]any) (CallToolResult, error) {
108167
name, _ := args["name"].(string)
109168
if name == "" {

kernel/mcp/tools/skill_test.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// SiYuan - From thought to insight, with agents
2+
// Copyright (c) 2020-present, b3log.org
3+
//
4+
// This program is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU Affero General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
9+
package tools
10+
11+
import (
12+
"os"
13+
"path/filepath"
14+
"strings"
15+
"testing"
16+
17+
kernelConf "github.com/siyuan-note/siyuan/kernel/conf"
18+
"github.com/siyuan-note/siyuan/kernel/model"
19+
"github.com/siyuan-note/siyuan/kernel/util"
20+
)
21+
22+
func setSkillToolTestEnvironment(t *testing.T) string {
23+
t.Helper()
24+
originalWorkspaceDir, originalDataDir := util.WorkspaceDir, util.DataDir
25+
originalHomeDir, originalConfDir := util.HomeDir, util.ConfDir
26+
originalConf := model.Conf
27+
28+
root := t.TempDir()
29+
util.WorkspaceDir = filepath.Join(root, "workspace")
30+
util.DataDir = filepath.Join(util.WorkspaceDir, "data")
31+
util.HomeDir = filepath.Join(root, "home")
32+
util.ConfDir = filepath.Join(util.WorkspaceDir, "conf")
33+
model.Conf = model.NewAppConf()
34+
model.Conf.Variables = &kernelConf.Variables{Items: []*kernelConf.Variable{{Name: "VALUE", Value: "resolved"}}}
35+
t.Cleanup(func() {
36+
util.WorkspaceDir, util.DataDir = originalWorkspaceDir, originalDataDir
37+
util.HomeDir, util.ConfDir = originalHomeDir, originalConfDir
38+
model.Conf = originalConf
39+
})
40+
return util.SkillsDir()
41+
}
42+
43+
func writeSkillToolTestFile(t *testing.T, file, content string) {
44+
t.Helper()
45+
if err := os.MkdirAll(filepath.Dir(file), 0755); err != nil {
46+
t.Fatal(err)
47+
}
48+
if err := os.WriteFile(file, []byte(content), 0644); err != nil {
49+
t.Fatal(err)
50+
}
51+
}
52+
53+
func TestSkillLoadResolvesVariablesOnlyInInstructions(t *testing.T) {
54+
skillsRoot := setSkillToolTestEnvironment(t)
55+
skillDir := filepath.Join(skillsRoot, "skill-dir")
56+
writeSkillToolTestFile(t, filepath.Join(skillDir, "SKILL.md"),
57+
"---\nname: A&B\ndescription: description\n---\nbody {{vars.VALUE}}")
58+
writeSkillToolTestFile(t, filepath.Join(skillDir, "references", "spec&notes.md"), "{{vars.VALUE}}\n")
59+
60+
activation, err := skillLoad(map[string]any{"name": "A&B"})
61+
if err != nil || activation.IsError {
62+
t.Fatalf("skillLoad(activation) = %#v, %v", activation, err)
63+
}
64+
activationText := activation.Content[0].Text
65+
for _, expected := range []string{
66+
`<skill_content name="A&amp;B">`,
67+
"body resolved",
68+
"<file>references/spec&amp;notes.md</file>",
69+
`<skill_location path="data/storage/ai/agent/skills/skill-dir">`,
70+
"use the `file` tool's read-only actions: `read`, `list`, `grep`, and `find`",
71+
} {
72+
if !strings.Contains(activationText, expected) {
73+
t.Errorf("activation output is missing %q:\n%s", expected, activationText)
74+
}
75+
}
76+
77+
resource, err := skillLoad(map[string]any{"name": "A&B/references/spec&notes.md"})
78+
if err != nil || resource.IsError {
79+
t.Fatalf("skillLoad(resource) = %#v, %v", resource, err)
80+
}
81+
resourceText := resource.Content[0].Text
82+
if !strings.Contains(resourceText, `<skill_resource skill="A&amp;B" path="references/spec&amp;notes.md">`) ||
83+
!strings.Contains(resourceText, "{{vars.VALUE}}") {
84+
t.Fatalf("unexpected resource output:\n%s", resourceText)
85+
}
86+
for _, excluded := range []string{"<skill_resources", "<skill_location", "resolved"} {
87+
if strings.Contains(resourceText, excluded) {
88+
t.Errorf("resource output unexpectedly contains %q:\n%s", excluded, resourceText)
89+
}
90+
}
91+
92+
truncated := formatSkillContent(&util.SkillLoadResult{Name: "truncated", ResourcesTruncated: true})
93+
if !strings.Contains(truncated, `<skill_resources truncated="true">`) {
94+
t.Fatalf("truncated empty manifest is not reported:\n%s", truncated)
95+
}
96+
}
97+
98+
func TestSkillContentOmitsLocationForWorkspaceSymlinkEscape(t *testing.T) {
99+
skillsRoot := setSkillToolTestEnvironment(t)
100+
externalSkill := filepath.Join(t.TempDir(), "external-skill")
101+
writeSkillToolTestFile(t, filepath.Join(externalSkill, "SKILL.md"),
102+
"---\nname: external\ndescription: description\n---\nbody")
103+
if err := os.MkdirAll(skillsRoot, 0755); err != nil {
104+
t.Fatal(err)
105+
}
106+
linkedSkill := filepath.Join(skillsRoot, "external")
107+
if err := os.Symlink(externalSkill, linkedSkill); err != nil {
108+
t.Skipf("symlinks are unavailable: %v", err)
109+
}
110+
111+
loaded, err := util.LoadSkill("external", nil)
112+
if err != nil {
113+
t.Fatal(err)
114+
}
115+
output := formatSkillContent(loaded)
116+
if strings.Contains(output, "<skill_location") {
117+
t.Fatalf("symlinked external skill exposed a file location:\n%s", output)
118+
}
119+
}

0 commit comments

Comments
 (0)