Files
zero/internal/tools/skill.go
363c0312d6 Skills & plugins: skill tool, skills loader, plugin manifests (#123)
* 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]>
2026-06-07 21:35:11 +05:30

81 lines
2.7 KiB
Go

package tools
import (
"context"
"fmt"
"strings"
"github.com/Gitlawb/zero/internal/skills"
)
// skillTool lets the model pull a reusable instruction "skill" into context on
// demand (PRD F15). It reads the skills directory itself via the internal/skills
// loader and returns the named skill's markdown body as its Output, so the model
// can opt into reusable guidance only when relevant. It is read-only.
type skillTool struct {
baseTool
dir string
}
// NewSkillTool builds the skill tool. An empty dir resolves to the standard
// skills data directory (skills.DefaultDir); pass an explicit dir in tests.
func NewSkillTool(dir string) *skillTool {
if strings.TrimSpace(dir) == "" {
dir = skills.DefaultDir(nil)
}
return &skillTool{
dir: dir,
baseTool: baseTool{
name: "skill",
description: "Load a named Zero skill and return its instructions as the tool output. " +
"Skills are reusable, on-demand instruction sets (e.g. project conventions or confirmation policies). " +
"Call this when a relevant skill exists; an unknown name returns the list of available skills.",
parameters: Schema{
Type: "object",
Properties: map[string]PropertySchema{
"name": {
Type: "string",
Description: "The name of the skill to load.",
},
"skill": {
Type: "string",
Description: "Alias for name; supply either name or skill.",
},
},
// Intentionally no strict Required: the tool needs exactly one of
// name/skill, which Run enforces via aliasedStringArg. Declaring both
// here keeps the alias usable under schema validators that reject
// unknown keys (AdditionalProperties:false).
AdditionalProperties: false,
},
safety: readOnlySafety("Reads a local skill file; gathers reusable instructions only."),
},
}
}
// Run loads the named skill and returns its Content. Unknown names return a
// clear error listing the available skill names so the model can self-correct.
func (tool *skillTool) Run(_ context.Context, args map[string]any) Result {
name, err := aliasedStringArg(args, []string{"name", "skill"}, "", true, false)
if err != nil {
return errorResult("Error: Invalid arguments for skill: " + err.Error())
}
loaded, err := skills.Load(tool.dir)
if err != nil {
return errorResult("Error: failed to load skills: " + err.Error())
}
if len(loaded) == 0 {
return errorResult(fmt.Sprintf("Error: no skills are available (looked in %s).", tool.dir))
}
names := make([]string, 0, len(loaded))
for _, skill := range loaded {
if skill.Name == name {
return okResult(skill.Content)
}
names = append(names, skill.Name)
}
return errorResult(fmt.Sprintf("Error: unknown skill %q. Available skills: %s.", name, strings.Join(names, ", ")))
}