mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-08 09:49:34 -10:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a4578dacc0 | ||
|
|
164150633a |
No files matched your search
@@ -110,6 +110,8 @@ Discovery roots (earlier wins on name collisions):
|
||||
|
||||
A missing directory is fine — Zero just omits it. Management commands (`zero skill add` / `zero skill remove`) always write to the primary Zero dir only; `list` / `info` search primary + `~/.agents/skills`. `skill info` reports lock source/hash when present and whether the on-disk `SKILL.md` has drifted from the recorded hash.
|
||||
|
||||
For a skill installed since Zero started installing bundled files, `skill info` also reports drift in any file installed with it.
|
||||
|
||||
```text
|
||||
~/.local/share/zero/skills/ # primary (install/remove target)
|
||||
run-benchmarks/
|
||||
@@ -140,6 +142,15 @@ Only `name` and `description` are recognized in the frontmatter today. The `name
|
||||
|
||||
The `skill` tool lets the agent load any discovered skill by name (primary, agents, or plugin).
|
||||
|
||||
A skill can ship files beside its `SKILL.md`, in the [agentskills.io](https://agentskills.io/specification) layout: `scripts/`, `references/`, `assets/` or anything else the instructions refer to. `zero skill add` installs the whole skill directory, leaving out hidden entries such as `.git` and not following or copying symlinks (it lists any it skipped). A skill larger than 1,000 files or 32 MiB is refused. When the `skill` tool loads a skill that has such files, its output ends with the skill's directory and a list of the files, so instructions like "run `scripts/fill.py`" can be followed. Running a bundled script goes through the normal tool permissions; installing or loading a skill never runs anything.
|
||||
|
||||
```text
|
||||
pdf/
|
||||
SKILL.md
|
||||
scripts/fill.py
|
||||
references/form-fields.md
|
||||
```
|
||||
|
||||
## 4. Hooks
|
||||
|
||||
Hooks fire shell commands on lifecycle events. Configure them in JSON:
|
||||
|
||||
@@ -92,13 +92,25 @@ func runSkillAdd(args []string, dir string, stdout io.Writer, stderr io.Writer)
|
||||
lines := []string{}
|
||||
if result.Updated {
|
||||
lines = append(lines, fmt.Sprintf("Updated skill %q.", result.Name))
|
||||
lines = append(lines, fmt.Sprintf(" hash: %s -> %s", shortHash(result.PreviousHash), shortHash(result.Hash)))
|
||||
switch {
|
||||
case result.PreviousHash != result.Hash:
|
||||
lines = append(lines, fmt.Sprintf(" hash: %s -> %s", shortHash(result.PreviousHash), shortHash(result.Hash)))
|
||||
case result.PreviousTreeHash == "":
|
||||
// The install this replaced predates tree hashes and copied SKILL.md
|
||||
// alone, so the bundled files are new rather than changed.
|
||||
lines = append(lines, " SKILL.md is unchanged; the files bundled with it are now installed too")
|
||||
default:
|
||||
lines = append(lines, " SKILL.md is unchanged; the files bundled with it changed")
|
||||
}
|
||||
} else {
|
||||
lines = append(lines, fmt.Sprintf("Installed skill %q.", result.Name))
|
||||
lines = append(lines, " hash: "+result.Hash)
|
||||
}
|
||||
lines = append(lines, " source: "+result.Source)
|
||||
lines = append(lines, " path: "+result.Path)
|
||||
if len(result.SkippedLinks) > 0 {
|
||||
lines = append(lines, " not copied (symlinks): "+strings.Join(result.SkippedLinks, ", "))
|
||||
}
|
||||
if _, err := fmt.Fprintln(stdout, redaction.RedactString(strings.Join(lines, "\n"), redaction.Options{})); err != nil {
|
||||
return exitCrash
|
||||
}
|
||||
|
||||
@@ -7,6 +7,8 @@ import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/Twigpine/zero/internal/skills"
|
||||
)
|
||||
|
||||
func writeSourceSkillDir(t *testing.T, dir string, content string) string {
|
||||
@@ -391,3 +393,133 @@ func TestRunDistributionCommandsDoNotLaunchTUI(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A reinstall that only changed a bundled script must still read as an update
|
||||
// (#584): the SKILL.md hash is the same, so "hash: X -> X" would say nothing.
|
||||
func TestRunSkillAddReportsAnUpdateToBundledFilesOnly(t *testing.T) {
|
||||
skillsDir := t.TempDir()
|
||||
src := writeSourceSkillDir(t, filepath.Join(t.TempDir(), "src"),
|
||||
"---\nname: pdf\ndescription: Fill forms.\n---\nRun scripts/fill.py.\n")
|
||||
script := filepath.Join(src, "scripts", "fill.py")
|
||||
if err := os.MkdirAll(filepath.Dir(script), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(script, []byte("print('v1')\n"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
deps := appDeps{skillsDir: func() string { return skillsDir }}
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
if exit := runWithDeps([]string{"skill", "add", src}, &stdout, &stderr, deps); exit != 0 {
|
||||
t.Fatalf("first skill add exit = %d, stderr = %s", exit, stderr.String())
|
||||
}
|
||||
if err := os.WriteFile(script, []byte("print('v2')\n"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
stdout.Reset()
|
||||
stderr.Reset()
|
||||
if exit := runWithDeps([]string{"skill", "add", src}, &stdout, &stderr, deps); exit != 0 {
|
||||
t.Fatalf("second skill add exit = %d, stderr = %s", exit, stderr.String())
|
||||
}
|
||||
for _, want := range []string{`Updated skill "pdf".`, "SKILL.md is unchanged; the files bundled with it changed"} {
|
||||
if !strings.Contains(stdout.String(), want) {
|
||||
t.Fatalf("second add output does not say %q:\n%s", want, stdout.String())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// An install from before tree hashes copied SKILL.md alone and recorded no tree
|
||||
// hash. Adding the skill again over that entry must not claim bundled files
|
||||
// changed: there are none, or they are installed for the first time.
|
||||
func TestRunSkillAddOverAnEntryWithoutATreeHash(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
script bool
|
||||
want []string
|
||||
notWant []string
|
||||
}{
|
||||
{
|
||||
name: "SKILL.md only",
|
||||
want: []string{`Installed skill "pdf".`},
|
||||
notWant: []string{"Updated skill", "files bundled with it"},
|
||||
},
|
||||
{
|
||||
name: "with a bundled script",
|
||||
script: true,
|
||||
want: []string{`Updated skill "pdf".`, "SKILL.md is unchanged; the files bundled with it are now installed too"},
|
||||
notWant: []string{"files bundled with it changed"},
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
skillsDir := t.TempDir()
|
||||
src := writeSourceSkillDir(t, filepath.Join(t.TempDir(), "src"),
|
||||
"---\nname: pdf\ndescription: Fill forms.\n---\nRun scripts/fill.py.\n")
|
||||
if tc.script {
|
||||
script := filepath.Join(src, "scripts", "fill.py")
|
||||
if err := os.MkdirAll(filepath.Dir(script), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(script, []byte("print('fill')\n"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
deps := appDeps{skillsDir: func() string { return skillsDir }}
|
||||
|
||||
var stdout, stderr bytes.Buffer
|
||||
if exit := runWithDeps([]string{"skill", "add", src}, &stdout, &stderr, deps); exit != 0 {
|
||||
t.Fatalf("first skill add exit = %d, stderr = %s", exit, stderr.String())
|
||||
}
|
||||
lock, err := skills.ReadLock(skillsDir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
entry := lock["pdf"]
|
||||
entry.TreeHash = ""
|
||||
lock["pdf"] = entry
|
||||
data, err := json.Marshal(lock)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(skillsDir, skills.LockFileName), data, 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
stdout.Reset()
|
||||
stderr.Reset()
|
||||
if exit := runWithDeps([]string{"skill", "add", src}, &stdout, &stderr, deps); exit != 0 {
|
||||
t.Fatalf("second skill add exit = %d, stderr = %s", exit, stderr.String())
|
||||
}
|
||||
for _, want := range tc.want {
|
||||
if !strings.Contains(stdout.String(), want) {
|
||||
t.Fatalf("second add output does not say %q:\n%s", want, stdout.String())
|
||||
}
|
||||
}
|
||||
for _, notWant := range tc.notWant {
|
||||
if strings.Contains(stdout.String(), notWant) {
|
||||
t.Fatalf("second add output says %q:\n%s", notWant, stdout.String())
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunSkillAddListsTheSymlinksItDidNotCopy(t *testing.T) {
|
||||
skillsDir := t.TempDir()
|
||||
src := writeSourceSkillDir(t, filepath.Join(t.TempDir(), "src"),
|
||||
"---\nname: pdf\ndescription: Fill forms.\n---\nbody\n")
|
||||
outside := filepath.Join(t.TempDir(), "notes.md")
|
||||
if err := os.WriteFile(outside, []byte("outside\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Symlink(outside, filepath.Join(src, "notes.md")); err != nil {
|
||||
t.Skipf("symlinks unavailable: %v", err)
|
||||
}
|
||||
var stdout, stderr bytes.Buffer
|
||||
deps := appDeps{skillsDir: func() string { return skillsDir }}
|
||||
if exit := runWithDeps([]string{"skill", "add", src}, &stdout, &stderr, deps); exit != 0 {
|
||||
t.Fatalf("skill add exit = %d, stderr = %s", exit, stderr.String())
|
||||
}
|
||||
if !strings.Contains(stdout.String(), "not copied (symlinks): notes.md") {
|
||||
t.Fatalf("add output does not list the skipped symlink:\n%s", stdout.String())
|
||||
}
|
||||
}
|
||||
@@ -374,7 +374,7 @@ func (tool skillTool) Run(_ context.Context, args map[string]any) tools.Result {
|
||||
available := make([]string, 0, len(merged))
|
||||
for _, skill := range merged {
|
||||
if skill.Name == name {
|
||||
return tools.Result{Status: tools.StatusOK, Output: skill.Content}
|
||||
return tools.Result{Status: tools.StatusOK, Output: skills.ToolOutput(skill)}
|
||||
}
|
||||
available = append(available, skill.Name)
|
||||
}
|
||||
|
||||
@@ -673,3 +673,28 @@ func TestExpandPluginRootPathRelative(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The multi-root skill tool is the one the CLI registers, so it has to point at
|
||||
// a skill's bundled files the same way the core tool does (#584).
|
||||
func TestNewSkillToolPointsAtTheSkillsBundledFiles(t *testing.T) {
|
||||
home := t.TempDir()
|
||||
t.Setenv("HOME", home)
|
||||
t.Setenv("USERPROFILE", home)
|
||||
dir := t.TempDir()
|
||||
writeTestSkill(t, dir, "pdf", "Run scripts/fill.py.")
|
||||
script := filepath.Join(dir, "pdf", "scripts", "fill.py")
|
||||
if err := os.MkdirAll(filepath.Dir(script), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(script, []byte("print('fill')\n"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
got := NewSkillTool(dir, nil).Run(context.Background(), map[string]any{"name": "pdf"})
|
||||
if got.Status != tools.StatusOK {
|
||||
t.Fatalf("Status = %s (output: %s)", got.Status, got.Output)
|
||||
}
|
||||
if !strings.Contains(got.Output, "- scripts/fill.py") || !strings.Contains(got.Output, filepath.Join(dir, "pdf")) {
|
||||
t.Fatalf("output does not point at the bundled script:\n%s", got.Output)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,258 @@
|
||||
package skills
|
||||
|
||||
import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"io/fs"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// A skill is a directory, not just its SKILL.md: the instructions often point at
|
||||
// scripts, references or assets that sit beside them (the agentskills.io
|
||||
// layout). Install copies that directory, and the skill tool says where it is.
|
||||
|
||||
// Install refuses a skill tree larger than this. A skill is a handful of
|
||||
// scripts and documents; a source that is really a whole repository or a pile
|
||||
// of binaries should fail loudly instead of filling the skills directory.
|
||||
var (
|
||||
maxSkillTreeFiles = 1000
|
||||
maxSkillTreeBytes = int64(32 << 20)
|
||||
)
|
||||
|
||||
// bundledFileListLimit caps how many bundled files the skill tool lists, so a
|
||||
// large skill does not crowd the instructions out of the tool output.
|
||||
const bundledFileListLimit = 50
|
||||
|
||||
// skipSkillEntry reports whether a skill tree entry is left out of an install,
|
||||
// a tree hash and a listing alike. Hidden entries cover VCS metadata (.git,
|
||||
// which a cloned source always has) and local clutter such as .DS_Store or
|
||||
// .env; they are not part of what a skill ships.
|
||||
func skipSkillEntry(name string) bool {
|
||||
return strings.HasPrefix(name, ".")
|
||||
}
|
||||
|
||||
// copySkillTree copies the skill directory src into dst, which must exist.
|
||||
// Regular files are copied with normalized permissions (0o755 when the source
|
||||
// is executable, 0o644 otherwise), so a script stays runnable without carrying
|
||||
// over group- or world-writable bits. Symlinks are not followed or copied:
|
||||
// their relative paths come back in skipped, so the caller can say so. Other
|
||||
// non-regular entries (devices, sockets, pipes) are left out. bundled counts
|
||||
// the files copied besides SKILL.md.
|
||||
func copySkillTree(src, dst string) (bundled int, skipped []string, err error) {
|
||||
files := 0
|
||||
var total int64
|
||||
err = filepath.WalkDir(src, func(path string, entry fs.DirEntry, walkErr error) error {
|
||||
if walkErr != nil {
|
||||
return walkErr
|
||||
}
|
||||
if path == src {
|
||||
return nil
|
||||
}
|
||||
rel, err := filepath.Rel(src, path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if skipSkillEntry(entry.Name()) {
|
||||
if entry.IsDir() {
|
||||
return filepath.SkipDir
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if entry.Type()&fs.ModeSymlink != 0 {
|
||||
skipped = append(skipped, filepath.ToSlash(rel))
|
||||
return nil
|
||||
}
|
||||
target := filepath.Join(dst, rel)
|
||||
if entry.IsDir() {
|
||||
return os.MkdirAll(target, 0o755)
|
||||
}
|
||||
if !entry.Type().IsRegular() {
|
||||
return nil
|
||||
}
|
||||
info, err := entry.Info()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
files++
|
||||
total += info.Size()
|
||||
if files > maxSkillTreeFiles {
|
||||
return fmt.Errorf("skill has more than %d files; a skill directory should hold only what the skill uses", maxSkillTreeFiles)
|
||||
}
|
||||
if total > maxSkillTreeBytes {
|
||||
return fmt.Errorf("skill is larger than %d MiB; a skill directory should hold only what the skill uses", maxSkillTreeBytes>>20)
|
||||
}
|
||||
if filepath.ToSlash(rel) != skillFileName {
|
||||
bundled++
|
||||
}
|
||||
mode := os.FileMode(0o644)
|
||||
if info.Mode().Perm()&0o111 != 0 {
|
||||
mode = 0o755
|
||||
}
|
||||
return copySkillFile(path, target, mode)
|
||||
})
|
||||
if err != nil {
|
||||
return 0, nil, err
|
||||
}
|
||||
return bundled, skipped, nil
|
||||
}
|
||||
|
||||
func copySkillFile(src, dst string, mode os.FileMode) error {
|
||||
in, err := os.Open(src)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer in.Close()
|
||||
out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if _, err := io.Copy(out, in); err != nil {
|
||||
_ = out.Close()
|
||||
return err
|
||||
}
|
||||
if err := out.Close(); err != nil {
|
||||
return err
|
||||
}
|
||||
// The create mode is filtered by the umask; set it outright so an
|
||||
// executable script is still executable wherever the install ran.
|
||||
return os.Chmod(dst, mode)
|
||||
}
|
||||
|
||||
// skillTreeHash hashes every file a skill ships, with the same entries
|
||||
// copySkillTree copies: its relative path, whether it is executable, and its
|
||||
// content. Sorted by path, so the hash depends only on the tree.
|
||||
func skillTreeHash(dir string) (string, error) {
|
||||
type entry struct {
|
||||
rel string
|
||||
exec bool
|
||||
sum string
|
||||
}
|
||||
var entries []entry
|
||||
err := filepath.WalkDir(dir, func(path string, item fs.DirEntry, walkErr error) error {
|
||||
if walkErr != nil {
|
||||
return walkErr
|
||||
}
|
||||
if path == dir {
|
||||
return nil
|
||||
}
|
||||
if skipSkillEntry(item.Name()) {
|
||||
if item.IsDir() {
|
||||
return filepath.SkipDir
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if !item.Type().IsRegular() {
|
||||
return nil
|
||||
}
|
||||
info, err := item.Info()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
rel, err := filepath.Rel(dir, path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
sum := sha256.Sum256(data)
|
||||
entries = append(entries, entry{rel: filepath.ToSlash(rel), exec: info.Mode().Perm()&0o111 != 0, sum: hex.EncodeToString(sum[:])})
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
sort.Slice(entries, func(i, j int) bool { return entries[i].rel < entries[j].rel })
|
||||
hash := sha256.New()
|
||||
for _, item := range entries {
|
||||
flag := "-"
|
||||
if item.exec {
|
||||
flag = "x"
|
||||
}
|
||||
// Length-prefixed, so no path can be read as the start of another entry.
|
||||
fmt.Fprintf(hash, "%d:%s|%s|%s;", len(item.rel), item.rel, flag, item.sum)
|
||||
}
|
||||
return "sha256:" + hex.EncodeToString(hash.Sum(nil)), nil
|
||||
}
|
||||
|
||||
// BundledFiles lists the files a skill ships besides its SKILL.md, as
|
||||
// slash-separated paths relative to the skill's directory, sorted. Hidden
|
||||
// entries and symlinks are left out: the listing tells the model what the
|
||||
// skill brings, and is not a way to reach anything outside it. total is the
|
||||
// number of files found; at most limit come back.
|
||||
func BundledFiles(skill Skill, limit int) (files []string, total int, err error) {
|
||||
dir := filepath.Dir(skill.Path)
|
||||
if strings.TrimSpace(skill.Path) == "" {
|
||||
return nil, 0, errors.New("skill has no path")
|
||||
}
|
||||
err = filepath.WalkDir(dir, func(path string, entry fs.DirEntry, walkErr error) error {
|
||||
if walkErr != nil {
|
||||
return walkErr
|
||||
}
|
||||
if path == dir {
|
||||
return nil
|
||||
}
|
||||
if skipSkillEntry(entry.Name()) {
|
||||
if entry.IsDir() {
|
||||
return filepath.SkipDir
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if !entry.Type().IsRegular() {
|
||||
return nil
|
||||
}
|
||||
rel, err := filepath.Rel(dir, path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
rel = filepath.ToSlash(rel)
|
||||
if rel == skillFileName {
|
||||
return nil
|
||||
}
|
||||
total++
|
||||
if total > maxSkillTreeFiles {
|
||||
// A directory this large is not something the listing should walk
|
||||
// to the end of; the count stops being exact past the cap.
|
||||
return filepath.SkipAll
|
||||
}
|
||||
files = append(files, rel)
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
return nil, 0, err
|
||||
}
|
||||
sort.Strings(files)
|
||||
if limit >= 0 && len(files) > limit {
|
||||
files = files[:limit]
|
||||
}
|
||||
return files, total, nil
|
||||
}
|
||||
|
||||
// ToolOutput is what the skill tool returns for a loaded skill: its SKILL.md
|
||||
// body and, when the skill ships other files, the directory they are in and
|
||||
// what they are, so instructions that say "run scripts/extract.py" can be
|
||||
// followed. A skill that is only a SKILL.md gets its body back unchanged.
|
||||
func ToolOutput(skill Skill) string {
|
||||
files, total, err := BundledFiles(skill, bundledFileListLimit)
|
||||
if err != nil || total == 0 {
|
||||
return skill.Content
|
||||
}
|
||||
var builder strings.Builder
|
||||
builder.WriteString(strings.TrimRight(skill.Content, "\n"))
|
||||
builder.WriteString("\n\n---\n")
|
||||
fmt.Fprintf(&builder, "This skill's files are in %s. Paths in the instructions above are relative to that directory.\n", filepath.Dir(skill.Path))
|
||||
for _, file := range files {
|
||||
builder.WriteString("- " + file + "\n")
|
||||
}
|
||||
if more := total - len(files); more > 0 {
|
||||
fmt.Fprintf(&builder, "- (and %d more)\n", more)
|
||||
}
|
||||
return builder.String()
|
||||
}
|
||||
@@ -0,0 +1,351 @@
|
||||
package skills
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
const bundledSkillManifest = "---\nname: pdf\ndescription: fill PDF forms\n---\nRun scripts/fill.py with the form path.\n"
|
||||
|
||||
// writeBundledSourceSkill lays down a skill in the agentskills.io shape: a
|
||||
// SKILL.md plus the scripts, references and assets it points at, and the hidden
|
||||
// clutter a cloned source carries.
|
||||
func writeBundledSourceSkill(t *testing.T, dir string) string {
|
||||
t.Helper()
|
||||
writeSourceSkill(t, dir, bundledSkillManifest)
|
||||
files := map[string]string{
|
||||
"scripts/fill.py": "print('fill')\n",
|
||||
"references/form.md": "# Form fields\n",
|
||||
"assets/logo.svg": "<svg/>\n",
|
||||
"LICENSE.txt": "MIT\n",
|
||||
".git/config": "[core]\n",
|
||||
".env": "TOKEN=local-only\n",
|
||||
"scripts/.cache/tmp": "scratch\n",
|
||||
}
|
||||
for rel, content := range files {
|
||||
path := filepath.Join(dir, filepath.FromSlash(rel))
|
||||
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
mode := os.FileMode(0o644)
|
||||
if strings.HasPrefix(rel, "scripts/") {
|
||||
mode = 0o755
|
||||
}
|
||||
if err := os.WriteFile(path, []byte(content), mode); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
return dir
|
||||
}
|
||||
|
||||
func TestInstallCopiesTheSkillDirectory(t *testing.T) {
|
||||
destDir := t.TempDir()
|
||||
src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
|
||||
|
||||
result, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
|
||||
if err != nil {
|
||||
t.Fatalf("Install: %v", err)
|
||||
}
|
||||
installed := filepath.Join(destDir, "pdf")
|
||||
for _, rel := range []string{"SKILL.md", "scripts/fill.py", "references/form.md", "assets/logo.svg", "LICENSE.txt"} {
|
||||
if _, err := os.Stat(filepath.Join(installed, filepath.FromSlash(rel))); err != nil {
|
||||
t.Errorf("%s was not installed: %v", rel, err)
|
||||
}
|
||||
}
|
||||
for _, rel := range []string{".git", ".env", "scripts/.cache"} {
|
||||
if _, err := os.Stat(filepath.Join(installed, filepath.FromSlash(rel))); err == nil {
|
||||
t.Errorf("hidden %s was installed; it is not part of the skill", rel)
|
||||
}
|
||||
}
|
||||
if !strings.HasPrefix(result.TreeHash, "sha256:") {
|
||||
t.Fatalf("TreeHash = %q, want a sha256 tree hash", result.TreeHash)
|
||||
}
|
||||
lock, err := ReadLock(destDir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if lock["pdf"].TreeHash != result.TreeHash || lock["pdf"].Hash != result.Hash {
|
||||
t.Fatalf("lock entry = %+v, want the install's hashes %q and %q", lock["pdf"], result.Hash, result.TreeHash)
|
||||
}
|
||||
if runtime.GOOS == "windows" {
|
||||
return
|
||||
}
|
||||
script, err := os.Stat(filepath.Join(installed, "scripts", "fill.py"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if script.Mode().Perm() != 0o755 {
|
||||
t.Errorf("installed script mode = %v, want 0755 so the skill can still run it", script.Mode().Perm())
|
||||
}
|
||||
reference, err := os.Stat(filepath.Join(installed, "references", "form.md"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if reference.Mode().Perm() != 0o644 {
|
||||
t.Errorf("installed reference mode = %v, want 0644", reference.Mode().Perm())
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstallSkipsSymlinksAndSaysSo(t *testing.T) {
|
||||
destDir := t.TempDir()
|
||||
src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
|
||||
outside := filepath.Join(t.TempDir(), "secret.txt")
|
||||
if err := os.WriteFile(outside, []byte("not part of the skill\n"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Symlink(outside, filepath.Join(src, "references", "linked.md")); err != nil {
|
||||
t.Skipf("symlinks unavailable: %v", err)
|
||||
}
|
||||
|
||||
result, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
|
||||
if err != nil {
|
||||
t.Fatalf("Install: %v", err)
|
||||
}
|
||||
if len(result.SkippedLinks) != 1 || result.SkippedLinks[0] != "references/linked.md" {
|
||||
t.Fatalf("SkippedLinks = %v, want [references/linked.md]", result.SkippedLinks)
|
||||
}
|
||||
if _, err := os.Lstat(filepath.Join(destDir, "pdf", "references", "linked.md")); err == nil {
|
||||
t.Fatal("a symlink from the source was installed")
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstallRefusesAnOversizedSkill(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
files int
|
||||
bytes int64
|
||||
want string
|
||||
}{
|
||||
{name: "too many files", files: 3, bytes: maxSkillTreeBytes, want: "more than 3 files"},
|
||||
{name: "too many bytes", files: maxSkillTreeFiles, bytes: 1 << 20, want: "larger than 1 MiB"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
files, bytes := maxSkillTreeFiles, maxSkillTreeBytes
|
||||
t.Cleanup(func() { maxSkillTreeFiles, maxSkillTreeBytes = files, bytes })
|
||||
maxSkillTreeFiles, maxSkillTreeBytes = tc.files, tc.bytes
|
||||
|
||||
destDir := t.TempDir()
|
||||
src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
|
||||
big := make([]byte, 2<<20)
|
||||
if err := os.WriteFile(filepath.Join(src, "assets", "big.bin"), big, 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
|
||||
if err == nil || !strings.Contains(err.Error(), tc.want) {
|
||||
t.Fatalf("Install error = %v, want %q", err, tc.want)
|
||||
}
|
||||
if _, statErr := os.Stat(filepath.Join(destDir, "pdf")); statErr == nil {
|
||||
t.Fatal("a refused install left a skill directory behind")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestReinstallWithOnlyABundledFileChangedIsAnUpdate(t *testing.T) {
|
||||
destDir := t.TempDir()
|
||||
src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
|
||||
first, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
|
||||
if err != nil {
|
||||
t.Fatalf("first Install: %v", err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(src, "scripts", "fill.py"), []byte("print('fill v2')\n"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
second, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
|
||||
if err != nil {
|
||||
t.Fatalf("second Install: %v", err)
|
||||
}
|
||||
if !second.Updated {
|
||||
t.Fatal("a reinstall that changed a bundled script was not reported as an update")
|
||||
}
|
||||
if second.Hash != first.Hash || second.TreeHash == first.TreeHash {
|
||||
t.Fatalf("hashes = %q/%q after %q/%q, want the same SKILL.md hash and a new tree hash", second.Hash, second.TreeHash, first.Hash, first.TreeHash)
|
||||
}
|
||||
if second.PreviousHash != first.Hash || second.PreviousTreeHash != first.TreeHash {
|
||||
t.Fatalf("previous hashes = %q/%q, want the first install's %q/%q", second.PreviousHash, second.PreviousTreeHash, first.Hash, first.TreeHash)
|
||||
}
|
||||
got, err := os.ReadFile(filepath.Join(destDir, "pdf", "scripts", "fill.py"))
|
||||
if err != nil || string(got) != "print('fill v2')\n" {
|
||||
t.Fatalf("installed script = %q, %v; want the new version", got, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInfoReportsDriftInABundledFile(t *testing.T) {
|
||||
destDir := t.TempDir()
|
||||
src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
|
||||
if _, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir}); err != nil {
|
||||
t.Fatalf("Install: %v", err)
|
||||
}
|
||||
if info, ok := Info(destDir, "pdf"); !ok || info.HashDrift {
|
||||
t.Fatalf("a fresh install reports drift: %+v, %v", info, ok)
|
||||
}
|
||||
script := filepath.Join(destDir, "pdf", "scripts", "fill.py")
|
||||
if err := os.WriteFile(script, []byte("print('edited by hand')\n"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if info, _ := Info(destDir, "pdf"); !info.HashDrift {
|
||||
t.Fatal("an edited bundled script is not reported as drift")
|
||||
}
|
||||
}
|
||||
|
||||
// A lock entry written before installs carried their bundled files has no tree
|
||||
// hash. Checking its tree would report every such skill as drifted, so only
|
||||
// its SKILL.md is checked, as before.
|
||||
func TestLockEntryWithoutATreeHashChecksOnlySKILLmd(t *testing.T) {
|
||||
destDir := t.TempDir()
|
||||
src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
|
||||
if _, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir}); err != nil {
|
||||
t.Fatalf("Install: %v", err)
|
||||
}
|
||||
dropTreeHash(t, destDir, "pdf")
|
||||
|
||||
if err := os.WriteFile(filepath.Join(destDir, "pdf", "scripts", "fill.py"), []byte("print('edited')\n"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if info, _ := Info(destDir, "pdf"); info.HashDrift {
|
||||
t.Fatal("an entry without a tree hash reported drift for a bundled file")
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(destDir, "pdf", skillFileName), []byte(bundledSkillManifest+"edited\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if info, _ := Info(destDir, "pdf"); !info.HashDrift {
|
||||
t.Fatal("an edited SKILL.md under an old lock entry is not reported as drift")
|
||||
}
|
||||
}
|
||||
|
||||
// Reinstalling over an entry from before tree hashes compares with what that
|
||||
// install left, SKILL.md alone: an unchanged skill that is only a SKILL.md is
|
||||
// not an update, and one whose bundled files this install adds is.
|
||||
func TestReinstallOverAnEntryWithoutATreeHash(t *testing.T) {
|
||||
t.Run("SKILL.md only", func(t *testing.T) {
|
||||
destDir := t.TempDir()
|
||||
src := writeSourceSkill(t, filepath.Join(t.TempDir(), "pdf"), bundledSkillManifest)
|
||||
if _, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir}); err != nil {
|
||||
t.Fatalf("first Install: %v", err)
|
||||
}
|
||||
dropTreeHash(t, destDir, "pdf")
|
||||
|
||||
result, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
|
||||
if err != nil {
|
||||
t.Fatalf("second Install: %v", err)
|
||||
}
|
||||
if result.Updated {
|
||||
t.Fatalf("reinstalling an unchanged SKILL.md-only skill over an old entry was reported as an update: %+v", result)
|
||||
}
|
||||
lock, err := ReadLock(destDir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if lock["pdf"].TreeHash != result.TreeHash {
|
||||
t.Fatalf("lock tree hash = %q after the reinstall, want %q", lock["pdf"].TreeHash, result.TreeHash)
|
||||
}
|
||||
})
|
||||
t.Run("with bundled files", func(t *testing.T) {
|
||||
destDir := t.TempDir()
|
||||
src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
|
||||
first, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
|
||||
if err != nil {
|
||||
t.Fatalf("first Install: %v", err)
|
||||
}
|
||||
dropTreeHash(t, destDir, "pdf")
|
||||
|
||||
result, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
|
||||
if err != nil {
|
||||
t.Fatalf("second Install: %v", err)
|
||||
}
|
||||
if !result.Updated || result.Hash != first.Hash || result.PreviousHash != first.Hash || result.PreviousTreeHash != "" {
|
||||
t.Fatalf("result = %+v, want an update with SKILL.md hash %q unchanged and no previous tree hash", result, first.Hash)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// dropTreeHash rewrites the lock entry for name the way an install from before
|
||||
// tree hashes left it: a source and a SKILL.md hash, nothing for the tree.
|
||||
func dropTreeHash(t *testing.T, destDir, name string) {
|
||||
t.Helper()
|
||||
lock, err := ReadLock(destDir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
entry := lock[name]
|
||||
entry.TreeHash = ""
|
||||
lock[name] = entry
|
||||
data, err := json.Marshal(lock)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(destDir, LockFileName), data, 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolOutputSaysWhereTheBundledFilesAre(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeBundledSourceSkill(t, filepath.Join(root, "pdf"))
|
||||
skill, ok := Get(root, "pdf")
|
||||
if !ok {
|
||||
t.Fatal("skill not loaded")
|
||||
}
|
||||
|
||||
output := ToolOutput(skill)
|
||||
if !strings.HasPrefix(output, strings.TrimRight(skill.Content, "\n")) {
|
||||
t.Fatalf("the instructions do not come first:\n%s", output)
|
||||
}
|
||||
if !strings.Contains(output, filepath.Dir(skill.Path)) {
|
||||
t.Fatalf("the output does not say where the skill's files are:\n%s", output)
|
||||
}
|
||||
for _, want := range []string{"- LICENSE.txt", "- assets/logo.svg", "- references/form.md", "- scripts/fill.py"} {
|
||||
if !strings.Contains(output, want+"\n") {
|
||||
t.Errorf("output does not list %q:\n%s", want, output)
|
||||
}
|
||||
}
|
||||
for _, hidden := range []string{".env", ".git", ".cache", "SKILL.md\n"} {
|
||||
if strings.Contains(output, "- "+hidden) {
|
||||
t.Errorf("output lists %q:\n%s", hidden, output)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolOutputCapsALongListing(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
dir := writeSourceSkill(t, filepath.Join(root, "many"), "---\nname: many\n---\nbody\n")
|
||||
for i := 0; i < bundledFileListLimit+7; i++ {
|
||||
path := filepath.Join(dir, "references", fmt.Sprintf("part-%03d.md", i))
|
||||
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(path, []byte("x\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
skill, ok := Get(root, "many")
|
||||
if !ok {
|
||||
t.Fatal("skill not loaded")
|
||||
}
|
||||
output := ToolOutput(skill)
|
||||
if got := strings.Count(output, "\n- references/"); got != bundledFileListLimit {
|
||||
t.Fatalf("listed %d files, want the first %d:\n%s", got, bundledFileListLimit, output)
|
||||
}
|
||||
if !strings.Contains(output, "- (and 7 more)\n") {
|
||||
t.Fatalf("output does not say how many more there are:\n%s", output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolOutputOfAManifestOnlySkillIsItsContent(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeSourceSkill(t, filepath.Join(root, "plain"), "---\nname: plain\n---\nJust instructions.\n")
|
||||
skill, ok := Get(root, "plain")
|
||||
if !ok {
|
||||
t.Fatal("skill not loaded")
|
||||
}
|
||||
if got := ToolOutput(skill); got != skill.Content {
|
||||
t.Fatalf("ToolOutput = %q, want the unchanged content %q", got, skill.Content)
|
||||
}
|
||||
}
|
||||
@@ -15,9 +15,10 @@ func Info(dir string, name string) (SkillInfo, bool) {
|
||||
if entry, found := lock[skill.Name]; found {
|
||||
info.Source = entry.Source
|
||||
info.Hash = entry.Hash
|
||||
info.TreeHash = entry.TreeHash
|
||||
}
|
||||
}
|
||||
info.HashDrift = skillHashDrift(skill, info.Hash)
|
||||
info.HashDrift = skillHashDrift(skill, info.Hash, info.TreeHash)
|
||||
return info, true
|
||||
}
|
||||
|
||||
|
||||
+93
-28
@@ -1,9 +1,9 @@
|
||||
package skills
|
||||
|
||||
// Distribution: install a skill from a git URL or a local path into the skills
|
||||
// directory, with a content hash recorded in a lockfile (skills.lock) so every
|
||||
// install/update is verifiable. Skills are markdown, so install NEVER executes
|
||||
// fetched content — it copies and validates the SKILL.md and nothing else.
|
||||
// directory, with content hashes recorded in a lockfile (skills.lock) so every
|
||||
// install/update is verifiable. Install copies the skill's directory, its
|
||||
// SKILL.md and the files beside it, and NEVER executes fetched content.
|
||||
|
||||
import (
|
||||
"context"
|
||||
@@ -61,16 +61,30 @@ type InstallResult struct {
|
||||
Hash string `json:"hash"`
|
||||
// Source echoes the source the skill was installed from.
|
||||
Source string `json:"source"`
|
||||
// TreeHash covers every file the skill ships, SKILL.md included; Hash is
|
||||
// still the SKILL.md hash alone.
|
||||
TreeHash string `json:"treeHash"`
|
||||
// Updated is true when an existing install was replaced; PreviousHash then
|
||||
// carries the prior recorded hash so a caller can show the change.
|
||||
Updated bool `json:"updated"`
|
||||
PreviousHash string `json:"previousHash,omitempty"`
|
||||
// PreviousTreeHash is the tree hash the replaced install recorded. It is
|
||||
// empty when that install predates tree hashes, and so copied SKILL.md
|
||||
// alone.
|
||||
PreviousTreeHash string `json:"previousTreeHash,omitempty"`
|
||||
// SkippedLinks lists symlinks in the source, relative to the skill
|
||||
// directory. Install neither follows nor copies them.
|
||||
SkippedLinks []string `json:"skippedLinks,omitempty"`
|
||||
}
|
||||
|
||||
// LockEntry records the source and content hash for one installed skill.
|
||||
type LockEntry struct {
|
||||
Source string `json:"source"`
|
||||
Hash string `json:"hash"`
|
||||
// TreeHash covers the whole installed skill directory. Entries written
|
||||
// before skills installed their bundled files have none, and are checked on
|
||||
// their SKILL.md alone.
|
||||
TreeHash string `json:"treeHash,omitempty"`
|
||||
}
|
||||
|
||||
// SkillInfo bundles a discovered skill with its recorded source and hash, for
|
||||
@@ -79,12 +93,14 @@ type SkillInfo struct {
|
||||
Skill Skill `json:"skill"`
|
||||
Source string `json:"source,omitempty"`
|
||||
Hash string `json:"hash,omitempty"`
|
||||
TreeHash string `json:"treeHash,omitempty"`
|
||||
HashDrift bool `json:"hashDrift"`
|
||||
}
|
||||
|
||||
// Install fetches the skill at options.Source and copies its SKILL.md into
|
||||
// options.Dir/<name>/, validating the frontmatter and recording a content hash
|
||||
// in the lockfile. A git URL is fetched via the (injectable) GitRunner into a
|
||||
// Install fetches the skill at options.Source and copies its directory into
|
||||
// options.Dir/<name>/: SKILL.md and the scripts, references and assets beside
|
||||
// it. It validates the frontmatter and records the SKILL.md hash and a hash of
|
||||
// the whole tree in the lockfile. A git URL is fetched via the (injectable) GitRunner into a
|
||||
// temp dir; a local path is read in place. Fetched content is never executed.
|
||||
func Install(ctx context.Context, options InstallOptions) (InstallResult, error) {
|
||||
source := strings.TrimSpace(options.Source)
|
||||
@@ -133,10 +149,11 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
|
||||
if err := os.MkdirAll(staged, 0o755); err != nil {
|
||||
return InstallResult{}, fmt.Errorf("create staged skill dir: %w", err)
|
||||
}
|
||||
stagedManifest := filepath.Join(staged, skillFileName)
|
||||
if err := os.WriteFile(stagedManifest, data, 0o644); err != nil {
|
||||
return InstallResult{}, fmt.Errorf("stage SKILL.md: %w", err)
|
||||
bundled, skippedLinks, err := copySkillTree(skillDir, staged)
|
||||
if err != nil {
|
||||
return InstallResult{}, fmt.Errorf("stage skill directory: %w", err)
|
||||
}
|
||||
stagedManifest := filepath.Join(staged, skillFileName)
|
||||
stagedData, err := os.ReadFile(stagedManifest)
|
||||
if err != nil || hashContent(stagedData) != hash {
|
||||
if err != nil {
|
||||
@@ -169,8 +186,13 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
|
||||
return InstallResult{}, fmt.Errorf("%w: %q is installed from %s (use --force to overwrite)", ErrNameClash, name, previous.Source)
|
||||
}
|
||||
|
||||
treeHash, err := skillTreeHash(staged)
|
||||
if err != nil {
|
||||
return InstallResult{}, fmt.Errorf("hash staged skill directory: %w", err)
|
||||
}
|
||||
|
||||
target := filepath.Join(dir, name)
|
||||
lock[name] = LockEntry{Source: source, Hash: hash}
|
||||
lock[name] = LockEntry{Source: source, Hash: hash, TreeHash: treeHash}
|
||||
if err := installtxn.CommitDir(target, staged, func() error {
|
||||
return writeLock(dir, lock)
|
||||
}); err != nil {
|
||||
@@ -178,14 +200,26 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
|
||||
}
|
||||
|
||||
result := InstallResult{
|
||||
Name: name,
|
||||
Path: filepath.Join(target, skillFileName),
|
||||
Hash: hash,
|
||||
Source: source,
|
||||
Name: name,
|
||||
Path: filepath.Join(target, skillFileName),
|
||||
Hash: hash,
|
||||
TreeHash: treeHash,
|
||||
Source: source,
|
||||
SkippedLinks: skippedLinks,
|
||||
}
|
||||
if existed {
|
||||
result.Updated = previous.Hash != hash
|
||||
result.PreviousHash = previous.Hash
|
||||
result.PreviousTreeHash = previous.TreeHash
|
||||
if previous.TreeHash == "" {
|
||||
// An entry from before tree hashes is from an install that copied
|
||||
// SKILL.md alone. Against that, this install is an update when
|
||||
// SKILL.md changed or it now brings bundled files; a skill that is
|
||||
// still only its unchanged SKILL.md is not.
|
||||
result.Updated = previous.Hash != hash || bundled > 0
|
||||
} else {
|
||||
// A changed script with the same SKILL.md is still an update.
|
||||
result.Updated = previous.Hash != hash || previous.TreeHash != treeHash
|
||||
}
|
||||
}
|
||||
return result, nil
|
||||
}
|
||||
@@ -193,8 +227,8 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
|
||||
// lockReconciler answers, for one interrupted workspace, which of the two trees
|
||||
// beside each other the lockfile records. A kill between the tree swap and the
|
||||
// lockfile publish leaves exactly what a kill after both leaves, so filesystem
|
||||
// shape cannot tell them apart and only the recorded hash can: the tree it
|
||||
// describes is the one the user was told they had.
|
||||
// shape cannot tell them apart and only the recorded hashes can: the tree they
|
||||
// describe is the one the user was told they had.
|
||||
func lockReconciler(dir string) installtxn.Reconciler {
|
||||
return func(name string, target string, backup string) (installtxn.Phase, error) {
|
||||
lock, err := ReadLock(dir)
|
||||
@@ -215,24 +249,45 @@ func lockReconciler(dir string) installtxn.Reconciler {
|
||||
// recorded one and recovery must not pick between them.
|
||||
return installtxn.PhaseUnknown, nil
|
||||
}
|
||||
targetHash, err := manifestHash(target)
|
||||
committed, err := treeMatchesEntry(target, entry)
|
||||
if err != nil {
|
||||
return installtxn.PhaseUnknown, err
|
||||
}
|
||||
if targetHash == entry.Hash {
|
||||
if committed {
|
||||
return installtxn.PhaseCommitted, nil
|
||||
}
|
||||
backupHash, err := manifestHash(backup)
|
||||
prePublish, err := treeMatchesEntry(backup, entry)
|
||||
if err != nil {
|
||||
return installtxn.PhaseUnknown, err
|
||||
}
|
||||
if backupHash == entry.Hash {
|
||||
if prePublish {
|
||||
return installtxn.PhasePrePublish, nil
|
||||
}
|
||||
return installtxn.PhaseUnknown, nil
|
||||
}
|
||||
}
|
||||
|
||||
// treeMatchesEntry reports whether the skill tree at path is the one entry
|
||||
// records. A reinstall that changes only a bundled file leaves SKILL.md the same
|
||||
// in both trees of an interrupted workspace, so an entry with a tree hash has to
|
||||
// match on that as well. An entry from before tree hashes records SKILL.md
|
||||
// alone, and that is all it can be matched on. As in manifestHash, a tree with
|
||||
// no SKILL.md is a non-match and any other read error is returned.
|
||||
func treeMatchesEntry(path string, entry LockEntry) (bool, error) {
|
||||
sum, err := manifestHash(path)
|
||||
if err != nil || sum != entry.Hash {
|
||||
return false, err
|
||||
}
|
||||
if entry.TreeHash == "" {
|
||||
return true, nil
|
||||
}
|
||||
tree, err := skillTreeHash(path)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return tree == entry.TreeHash, nil
|
||||
}
|
||||
|
||||
// manifestHash hashes the SKILL.md under path the same way an install records
|
||||
// it. A tree with no SKILL.md is a non-match rather than a failure: an install
|
||||
// killed mid-copy can leave one, and it simply is not the tree the lockfile
|
||||
@@ -346,9 +401,10 @@ func InfoFromRoots(primaryDir string, roots []string, name string) (SkillInfo, b
|
||||
if entry, found := lock[skill.Name]; found {
|
||||
info.Source = entry.Source
|
||||
info.Hash = entry.Hash
|
||||
info.TreeHash = entry.TreeHash
|
||||
}
|
||||
}
|
||||
info.HashDrift = skillHashDrift(skill, info.Hash)
|
||||
info.HashDrift = skillHashDrift(skill, info.Hash, info.TreeHash)
|
||||
return info, true
|
||||
}
|
||||
|
||||
@@ -556,11 +612,12 @@ func hashContent(data []byte) string {
|
||||
return "sha256:" + hex.EncodeToString(sum[:])
|
||||
}
|
||||
|
||||
// skillHashDrift reports whether the on-disk SKILL.md no longer matches the
|
||||
// lockfile hash recorded at install time. Missing lock hashes never count as
|
||||
// drift (agents-only / unlocked skills). A locked skill whose SKILL.md cannot
|
||||
// be read is treated as drifted — not as a clean match.
|
||||
func skillHashDrift(skill Skill, lockHash string) bool {
|
||||
// skillHashDrift reports whether the installed skill no longer matches the
|
||||
// lockfile hashes recorded at install time: the SKILL.md hash, and the tree
|
||||
// hash when the entry has one, so an edited script counts as drift too. Missing
|
||||
// lock hashes never count as drift (skills Zero did not install). A locked
|
||||
// skill that cannot be read is treated as drifted, not as a clean match.
|
||||
func skillHashDrift(skill Skill, lockHash string, lockTreeHash string) bool {
|
||||
lockHash = strings.TrimSpace(lockHash)
|
||||
if lockHash == "" {
|
||||
return false
|
||||
@@ -569,5 +626,13 @@ func skillHashDrift(skill Skill, lockHash string) bool {
|
||||
if err != nil {
|
||||
return true
|
||||
}
|
||||
return hashContent(data) != lockHash
|
||||
if hashContent(data) != lockHash {
|
||||
return true
|
||||
}
|
||||
lockTreeHash = strings.TrimSpace(lockTreeHash)
|
||||
if lockTreeHash == "" {
|
||||
return false
|
||||
}
|
||||
treeHash, err := skillTreeHash(filepath.Dir(skill.Path))
|
||||
return err != nil || treeHash != lockTreeHash
|
||||
}
|
||||
@@ -364,10 +364,11 @@ func TestInstallGitSourceUsesRunnerAndDoesNotExecuteContent(t *testing.T) {
|
||||
if _, err := os.Stat(filepath.Join(cloneRoot, "PWNED")); err == nil {
|
||||
t.Fatalf("install must never execute fetched content")
|
||||
}
|
||||
// The fetched executable must not be copied into the skills dir either (skills
|
||||
// are markdown — only SKILL.md is installed, not arbitrary fetched files).
|
||||
if _, err := os.Stat(filepath.Join(destDir, "remote", "install.sh")); err == nil {
|
||||
t.Fatalf("install must not copy fetched executable into the skills dir")
|
||||
// A script beside SKILL.md is part of the skill (#584), so it is installed,
|
||||
// and still only copied: running it is up to whoever uses the skill later,
|
||||
// through the normal tool permissions.
|
||||
if _, err := os.Stat(filepath.Join(destDir, "remote", "install.sh")); err != nil {
|
||||
t.Fatalf("the skill's bundled script was not installed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -440,10 +441,10 @@ func TestInfoReturnsFrontmatterSourceAndHash(t *testing.T) {
|
||||
|
||||
func TestSkillHashDriftUnreadableLockedPath(t *testing.T) {
|
||||
missing := filepath.Join(t.TempDir(), "missing", "SKILL.md")
|
||||
if !skillHashDrift(Skill{Path: missing}, "sha256:deadbeef") {
|
||||
if !skillHashDrift(Skill{Path: missing}, "sha256:deadbeef", "") {
|
||||
t.Fatal("expected drift when locked SKILL.md cannot be read")
|
||||
}
|
||||
if skillHashDrift(Skill{Path: missing}, "") {
|
||||
if skillHashDrift(Skill{Path: missing}, "", "") {
|
||||
t.Fatal("missing lock hash must not count as drift")
|
||||
}
|
||||
}
|
||||
@@ -861,6 +862,146 @@ func TestRecoveryResolvesEveryInterruptedUpdateStep(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
const (
|
||||
bundledScriptV1 = "print('v1')\n"
|
||||
bundledScriptV2 = "print('v2')\n"
|
||||
)
|
||||
|
||||
func writeBundledScript(t *testing.T, skillDir string, content string) string {
|
||||
t.Helper()
|
||||
path := filepath.Join(skillDir, "scripts", "run.py")
|
||||
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(path, []byte(content), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return path
|
||||
}
|
||||
|
||||
// plantInterruptedBundledUpdate is plantInterruptedAlphaUpdate for a reinstall
|
||||
// that changes only a bundled script, killed after the tree swap: alpha's
|
||||
// SKILL.md is the same in the live tree and the backup, and only the tree hash
|
||||
// in the lockfile tells them apart. published says whether the lockfile was
|
||||
// rewritten for the new tree before the kill (S4) or not (S3).
|
||||
func plantInterruptedBundledUpdate(t *testing.T, published bool) interruptedUpdate {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
source := writeSourceSkill(t, filepath.Join(t.TempDir(), "alpha"), oldAlphaSkill)
|
||||
writeBundledScript(t, source, bundledScriptV1)
|
||||
if _, err := Install(context.Background(), InstallOptions{Source: source, Dir: dir}); err != nil {
|
||||
t.Fatalf("seed alpha: %v", err)
|
||||
}
|
||||
gammaSource := writeSourceSkill(t, filepath.Join(t.TempDir(), "gamma"), gammaSkill)
|
||||
if _, err := Install(context.Background(), InstallOptions{Source: gammaSource, Dir: dir}); err != nil {
|
||||
t.Fatalf("seed gamma: %v", err)
|
||||
}
|
||||
|
||||
staged, _, err := installtxn.StageDir(dir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
workspace := filepath.Dir(staged)
|
||||
writeWorkspaceMarker(t, workspace, "alpha")
|
||||
writeSourceSkill(t, staged, oldAlphaSkill)
|
||||
writeBundledScript(t, staged, bundledScriptV2)
|
||||
|
||||
target := filepath.Join(dir, "alpha")
|
||||
if err := os.Rename(target, filepath.Join(workspace, "previous")); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Rename(staged, target); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if published {
|
||||
treeHash, err := skillTreeHash(target)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
publishLockEntry(t, dir, "alpha", LockEntry{Source: canonicalSource(source), Hash: hashContent([]byte(oldAlphaSkill)), TreeHash: treeHash})
|
||||
}
|
||||
return interruptedUpdate{dir: dir, workspace: workspace, oldSource: canonicalSource(source), newSource: canonicalSource(source)}
|
||||
}
|
||||
|
||||
// SKILL.md cannot say which tree of an interrupted bundled-only update the
|
||||
// lockfile records, since both carry the same one. The tree hash can: before the
|
||||
// publish it still describes the backup, after it the live tree.
|
||||
func TestRecoveryTellsABundledOnlyUpdateApartByTreeHash(t *testing.T) {
|
||||
steps := []struct {
|
||||
step string
|
||||
published bool
|
||||
want string
|
||||
}{
|
||||
{step: "S3", published: false, want: bundledScriptV1},
|
||||
{step: "S4", published: true, want: bundledScriptV2},
|
||||
}
|
||||
for _, step := range steps {
|
||||
for _, driver := range recoveryDrivers {
|
||||
t.Run(step.step+"/"+driver.name, func(t *testing.T) {
|
||||
update := plantInterruptedBundledUpdate(t, step.published)
|
||||
|
||||
if err := driver.run(t, update.dir); err != nil {
|
||||
t.Fatalf("%s over a recoverable state: %v", driver.name, err)
|
||||
}
|
||||
|
||||
target := filepath.Join(update.dir, "alpha")
|
||||
got, err := os.ReadFile(filepath.Join(target, "scripts", "run.py"))
|
||||
if err != nil || string(got) != step.want {
|
||||
t.Fatalf("recovered script = %q, %v; want %q", got, err, step.want)
|
||||
}
|
||||
lock, err := ReadLock(update.dir)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
tree, err := skillTreeHash(target)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if lock["alpha"].TreeHash != tree {
|
||||
t.Errorf("lock tree hash %q does not describe the recovered tree %q", lock["alpha"].TreeHash, tree)
|
||||
}
|
||||
if _, err := os.Stat(update.workspace); !os.IsNotExist(err) {
|
||||
t.Errorf("a resolved workspace must be cleared, got %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A live tree whose files cannot be read cannot be shown to be the one the
|
||||
// lockfile records, or not to be. Recovery reports it, as it does for a SKILL.md
|
||||
// it cannot read, and decides nothing until a later pass can read it.
|
||||
func TestRecoveryReportsALiveTreeItCannotHash(t *testing.T) {
|
||||
for _, driver := range recoveryDrivers {
|
||||
t.Run(driver.name, func(t *testing.T) {
|
||||
update := plantInterruptedBundledUpdate(t, false)
|
||||
lockBefore := readLockBytes(t, update.dir)
|
||||
dirBefore := treeSnapshot(t, update.dir)
|
||||
release := holdUnreadable(t, filepath.Join(update.dir, "alpha", "scripts", "run.py"))
|
||||
|
||||
err := driver.run(t, update.dir)
|
||||
release()
|
||||
if err == nil || !strings.Contains(err.Error(), "classify interrupted install alpha") {
|
||||
t.Fatalf("%s over a live tree it cannot hash: %v, want the classification error", driver.name, err)
|
||||
}
|
||||
assertSnapshotUnchanged(t, dirBefore, treeSnapshot(t, update.dir))
|
||||
if got := readLockBytes(t, update.dir); got != lockBefore {
|
||||
t.Errorf("the lockfile was rewritten by a caller that must have aborted:\n%s", got)
|
||||
}
|
||||
|
||||
// Once the tree can be read, the next pass resolves it: the lockfile
|
||||
// still describes the backup, so the backup comes back.
|
||||
if err := driver.run(t, update.dir); err != nil {
|
||||
t.Fatalf("%s once the tree is readable: %v", driver.name, err)
|
||||
}
|
||||
got, err := os.ReadFile(filepath.Join(update.dir, "alpha", "scripts", "run.py"))
|
||||
if err != nil || string(got) != bundledScriptV1 {
|
||||
t.Fatalf("recovered script = %q, %v; want %q", got, err, bundledScriptV1)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A recovery that could not be made is not the same as nothing to recover.
|
||||
// Installing over or reporting the removal of a tree that is still owed a
|
||||
// restore destroys the only copy of it, so the caller has to stop before it
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
//go:build !windows
|
||||
|
||||
package skills
|
||||
|
||||
import (
|
||||
"os"
|
||||
"sync"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// holdUnreadable takes every permission off path, so reading it fails until
|
||||
// release puts them back. Cleanup releases it too. Root reads a file whatever
|
||||
// its mode, so a test that needs the read to fail is skipped there.
|
||||
func holdUnreadable(t *testing.T, path string) (release func()) {
|
||||
t.Helper()
|
||||
if os.Geteuid() == 0 {
|
||||
t.Skip("root reads a file whatever its mode")
|
||||
}
|
||||
info, err := os.Stat(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.Chmod(path, 0); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var once sync.Once
|
||||
release = func() { once.Do(func() { _ = os.Chmod(path, info.Mode().Perm()) }) }
|
||||
t.Cleanup(release)
|
||||
return release
|
||||
}
|
||||
@@ -0,0 +1,28 @@
|
||||
//go:build windows
|
||||
|
||||
package skills
|
||||
|
||||
import (
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"golang.org/x/sys/windows"
|
||||
)
|
||||
|
||||
// holdUnreadable keeps path open with no sharing, so every other open of it
|
||||
// fails until release runs. Cleanup releases it too.
|
||||
func holdUnreadable(t *testing.T, path string) (release func()) {
|
||||
t.Helper()
|
||||
name, err := windows.UTF16PtrFromString(path)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
handle, err := windows.CreateFile(name, windows.GENERIC_READ, 0, nil, windows.OPEN_EXISTING, windows.FILE_ATTRIBUTE_NORMAL, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("hold %s open: %v", path, err)
|
||||
}
|
||||
var once sync.Once
|
||||
release = func() { once.Do(func() { _ = windows.CloseHandle(handle) }) }
|
||||
t.Cleanup(release)
|
||||
return release
|
||||
}
|
||||
@@ -936,8 +936,15 @@ func TestExecCommandInteractiveBlockSuggestsTTY(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestExecCommandTTYSkipsInteractiveBlock(t *testing.T) {
|
||||
tool := NewScopedExecCommandTool(t.TempDir(), nil, newExecSessionManager())
|
||||
// ssh to a bogus host fails fast on its own; the point is that the
|
||||
// The ssh can still be resolving the bogus host when the 250ms yield
|
||||
// returns, which leaves its session running in the test root. Stop it
|
||||
// before the root goes, and use execTestRoot for the moment Windows still
|
||||
// holds the directory after the stop.
|
||||
root := execTestRoot(t)
|
||||
manager := newExecSessionManager()
|
||||
t.Cleanup(func() { manager.StopAll() })
|
||||
tool := NewScopedExecCommandTool(root, nil, manager)
|
||||
// ssh to a bogus host fails on its own; the point is that the
|
||||
// interactive guard must not fire ahead of the tty path.
|
||||
result := tool.Run(context.Background(), map[string]any{
|
||||
"cmd": "ssh host.example.com", "tty": true, "yield_time_ms": 250,
|
||||
|
||||
@@ -73,7 +73,7 @@ func (tool *skillTool) Run(_ context.Context, args map[string]any) Result {
|
||||
names := make([]string, 0, len(loaded))
|
||||
for _, skill := range loaded {
|
||||
if skill.Name == name {
|
||||
return okResult(skill.Content)
|
||||
return okResult(skills.ToolOutput(skill))
|
||||
}
|
||||
names = append(names, skill.Name)
|
||||
}
|
||||
|
||||
@@ -96,3 +96,27 @@ func TestSkillToolNoSkillsAvailable(t *testing.T) {
|
||||
t.Fatalf("expected a no-skills message, got: %q", result.Output)
|
||||
}
|
||||
}
|
||||
|
||||
// A skill that ships files beside its SKILL.md (#584) is no use if the model
|
||||
// cannot find them, so the output names their directory and lists them.
|
||||
func TestSkillToolPointsAtTheSkillsBundledFiles(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
writeSkillFile(t, dir, "pdf", "---\nname: pdf\n---\nRun scripts/fill.py.")
|
||||
script := filepath.Join(dir, "pdf", "scripts", "fill.py")
|
||||
if err := os.MkdirAll(filepath.Dir(script), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(script, []byte("print('fill')\n"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
result := NewSkillTool(dir).Run(context.Background(), map[string]any{"name": "pdf"})
|
||||
if result.Status != StatusOK {
|
||||
t.Fatalf("Status = %s (output: %s)", result.Status, result.Output)
|
||||
}
|
||||
for _, want := range []string{"Run scripts/fill.py.", filepath.Join(dir, "pdf"), "- scripts/fill.py"} {
|
||||
if !strings.Contains(result.Output, want) {
|
||||
t.Fatalf("output does not contain %q:\n%s", want, result.Output)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user