mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-10 05:01:28 -10:00
* Skills & plugins: skill tool, skills loader, plugin manifests Module of the runtime-core split (off main, after #119 tools merged). Clean extract — internal/skills is new; internal/plugins had no drift on main. No new deps. - internal/skills: loads */SKILL.md frontmatter from the skills dir (resolves its own XDG dir); no YAML dependency. - skill tool (internal/tools/skill.go), read-only, registered in CoreReadOnlyTools (so it's in the agent core + the MCP read-only default). Path-safe: it Load()s all skills and matches by exact name — it never builds a path from the model-supplied arg, so there is no traversal surface. knownToolNames gains "skill" for the specialist cross-package invariant; CoreReadOnlyTools count test bumped 4->5. - internal/plugins: plugin manifest metadata enrichment (Author/License/Keywords/Interface). FORWARD code — the cli plugin-loading wiring lands later. MCP hardening (nested-schema + hung-server fix) is a SEPARATE follow-up PR. build/vet/-race/full-suite + GOOS=windows build green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Address #123 review (CodeRabbit): pointer metadata, skill schema alias, dir fallback, List test - plugins: Author/Interface are now *PluginAuthor/*PluginInterface so omitempty actually omits them — a non-pointer struct is never empty to encoding/json, so the old form emitted author:{}/interface:{} and changed the serialized shape of plugins that don't set them. parseAuthor/parseInterface return nil when all fields are empty; formatAuthor takes the pointer. (Major) - skill: schema now declares the 'skill' alias alongside 'name' (and drops strict Required) so the alias survives schema validators that reject unknown keys (AdditionalProperties:false); Run still enforces exactly-one-of via aliasedStringArg. (Major) - skills.DefaultDir: return "" instead of a relative ".local/share/..." path when neither XDG_DATA_HOME nor a resolvable HOME exists, so skills can't bind to the process CWD (load("") already no-ops). (Minor) - skills_test: TestListReturnsNamesAndDescriptions now asserts List strips Content so a listing can't silently leak full skill bodies. (Minor) build/vet/-race/full-suite + GOOS=windows build green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Address #123 review (@Vasanthdev2004): confine skill SKILL.md to the skills root skill is a permission-allow read-only core/MCP tool, so the loader must not follow a symlinked SKILL.md (or skill dir) out of the skills root and become an arbitrary-file reader. load() now resolves the skills root via EvalSymlinks and, for each entry, resolves SKILL.md through symlinks and verifies the real path stays under the root (confineSkillPath); an escaping or unreadable path is skipped rather than read. Mirrors the grep/read_file confinement. Legit skills under a symlinked root (e.g. macOS /tmp) still load because the root is resolved too. Regression: TestLoadSkipsSymlinkedSkillFileEscapingRoot (a SKILL.md symlinked to a secret outside the root is skipped, not read). build/vet/-race/full-suite + GOOS=windows build green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * Address #123 re-review (CodeRabbit): reject non-regular SKILL.md, single-trim metadata - skills: confineSkillPath now Lstats the resolved path and skips non-regular targets (directory/FIFO/device/socket). A FIFO/device named SKILL.md (or an in-root symlink to one) would otherwise make os.ReadFile block indefinitely — skill is a permission-allow tool over a user-controlled dir. Regression: TestLoadSkipsNonRegularSkillFile (unix-tagged; mkfifo SKILL.md, asserts Load skips it within a timeout rather than hanging). (Major) - plugins: coerceMetaStringSlice trims each item once into a local var instead of calling strings.TrimSpace twice. (Nitpick) build/vet/-race/full-suite + GOOS=windows build green. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> --------- Co-authored-by: KRATOS <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
99 lines
3.1 KiB
Go
99 lines
3.1 KiB
Go
package tools
|
|
|
|
import (
|
|
"context"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
func writeSkillFile(t *testing.T, dir string, name string, content string) {
|
|
t.Helper()
|
|
skillDir := filepath.Join(dir, name)
|
|
if err := os.MkdirAll(skillDir, 0o755); err != nil {
|
|
t.Fatalf("mkdir %s: %v", skillDir, err)
|
|
}
|
|
if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644); err != nil {
|
|
t.Fatalf("write SKILL.md: %v", err)
|
|
}
|
|
}
|
|
|
|
func TestSkillToolIsReadOnly(t *testing.T) {
|
|
tool := NewSkillTool(t.TempDir())
|
|
if tool.Name() != "skill" {
|
|
t.Fatalf("Name = %q, want skill", tool.Name())
|
|
}
|
|
if tool.Safety().SideEffect != SideEffectRead {
|
|
t.Fatalf("SideEffect = %s, want read", tool.Safety().SideEffect)
|
|
}
|
|
if tool.Safety().Permission != PermissionAllow {
|
|
t.Fatalf("Permission = %s, want allow", tool.Safety().Permission)
|
|
}
|
|
if tool.Parameters().Type != "object" {
|
|
t.Fatalf("schema type = %s, want object", tool.Parameters().Type)
|
|
}
|
|
}
|
|
|
|
func TestSkillToolReturnsContentForKnownSkill(t *testing.T) {
|
|
dir := t.TempDir()
|
|
writeSkillFile(t, dir, "confirmation-policy", "---\nname: confirmation-policy\ndescription: ask first\n---\n\n# Confirmation Policy\n\nAsk before risky actions.")
|
|
|
|
tool := NewSkillTool(dir)
|
|
result := tool.Run(context.Background(), map[string]any{"name": "confirmation-policy"})
|
|
if result.Status != StatusOK {
|
|
t.Fatalf("Status = %s, want ok (output: %s)", result.Status, result.Output)
|
|
}
|
|
if !strings.Contains(result.Output, "Ask before risky actions.") {
|
|
t.Fatalf("Output missing skill body: %q", result.Output)
|
|
}
|
|
}
|
|
|
|
func TestSkillToolAcceptsSkillAlias(t *testing.T) {
|
|
dir := t.TempDir()
|
|
writeSkillFile(t, dir, "demo", "body of demo")
|
|
|
|
tool := NewSkillTool(dir)
|
|
result := tool.Run(context.Background(), map[string]any{"skill": "demo"})
|
|
if result.Status != StatusOK {
|
|
t.Fatalf("Status = %s, want ok (output: %s)", result.Status, result.Output)
|
|
}
|
|
if !strings.Contains(result.Output, "body of demo") {
|
|
t.Fatalf("Output = %q", result.Output)
|
|
}
|
|
}
|
|
|
|
func TestSkillToolUnknownSkillErrorsAndListsAvailable(t *testing.T) {
|
|
dir := t.TempDir()
|
|
writeSkillFile(t, dir, "alpha", "a")
|
|
writeSkillFile(t, dir, "beta", "b")
|
|
|
|
tool := NewSkillTool(dir)
|
|
result := tool.Run(context.Background(), map[string]any{"name": "missing"})
|
|
if result.Status != StatusError {
|
|
t.Fatalf("Status = %s, want error", result.Status)
|
|
}
|
|
if !strings.Contains(result.Output, "alpha") || !strings.Contains(result.Output, "beta") {
|
|
t.Fatalf("error should list available skills, got: %q", result.Output)
|
|
}
|
|
}
|
|
|
|
func TestSkillToolMissingNameErrors(t *testing.T) {
|
|
tool := NewSkillTool(t.TempDir())
|
|
result := tool.Run(context.Background(), map[string]any{})
|
|
if result.Status != StatusError {
|
|
t.Fatalf("Status = %s, want error", result.Status)
|
|
}
|
|
}
|
|
|
|
func TestSkillToolNoSkillsAvailable(t *testing.T) {
|
|
tool := NewSkillTool(filepath.Join(t.TempDir(), "missing"))
|
|
result := tool.Run(context.Background(), map[string]any{"name": "anything"})
|
|
if result.Status != StatusError {
|
|
t.Fatalf("Status = %s, want error", result.Status)
|
|
}
|
|
if !strings.Contains(strings.ToLower(result.Output), "no skills") {
|
|
t.Fatalf("expected a no-skills message, got: %q", result.Output)
|
|
}
|
|
}
|