Merge branch 'main' into fix/skills-dialog-wrap
3 files · +251 -35
M docs/skills.md +8 -4
| @@ -74,10 +74,14 @@ Explicit `--ext` bundles take precedence over environment, project, global, and | |||
| 74 | 74 | compatibility sources. Duplicate names from different files keep the | |
| 75 | 75 | higher-precedence file and report a diagnostic. Rediscovering the same file | |
| 76 | 76 | under the same skill name (including through a symlink) is silently ignored. | |
| 77 | - | This includes overlapping project and global locations when running from your | |
| 78 | - | home directory. `--no-ext` omits implicit extension bundles; | |
| 79 | - | `--no-ext --ext PATH` loads only the explicitly named bundle. `--no-skill` | |
| 80 | - | disables all skills, including extension and built-in skills. | |
| 77 | + | Skill directories may themselves be symlinks, so a checkout can be kept in one | |
| 78 | + | place and linked into a skills directory. Real directories are scanned before | |
| 79 | + | links, so an alias cannot rename an existing unnamed skill. Link cycles are | |
| 80 | + | ignored, a directory reached twice through links is scanned once, and a link | |
| 81 | + | that points at nothing is skipped. This includes overlapping project and global | |
| 82 | + | locations when running from your home directory. `--no-ext` omits implicit | |
| 83 | + | extension bundles. `--no-ext --ext PATH` loads only the explicitly named bundle. | |
| 84 | + | `--no-skill` disables all skills, including extension and built-in skills. | |
| 81 | 85 | ||
| 82 | 86 | When `XDG_STATE_HOME` is set on any platform, `$ZOT_HOME` defaults to | |
| 83 | 87 | `$XDG_STATE_HOME/zot`. Otherwise it defaults to `~/Library/Application Support/zot/` | |
| @@ -74,10 +74,14 @@ Explicit `--ext` bundles take precedence over environment, project, global, and | |||||
| 74 | compatibility sources. Duplicate names from different files keep the | 74 | compatibility sources. Duplicate names from different files keep the | ||
| 75 | higher-precedence file and report a diagnostic. Rediscovering the same file | 75 | higher-precedence file and report a diagnostic. Rediscovering the same file | ||
| 76 | under the same skill name (including through a symlink) is silently ignored. | 76 | under the same skill name (including through a symlink) is silently ignored. | ||
| 77 | - | This includes overlapping project and global locations when running from your | 77 | + | Skill directories may themselves be symlinks, so a checkout can be kept in one |
| 78 | - | home directory. `--no-ext` omits implicit extension bundles; | 78 | + | place and linked into a skills directory. Real directories are scanned before |
| 79 | - | `--no-ext --ext PATH` loads only the explicitly named bundle. `--no-skill` | 79 | + | links, so an alias cannot rename an existing unnamed skill. Link cycles are |
| 80 | - | disables all skills, including extension and built-in skills. | 80 | + | ignored, a directory reached twice through links is scanned once, and a link |
| 81 | + | that points at nothing is skipped. This includes overlapping project and global | |||
| 82 | + | locations when running from your home directory. `--no-ext` omits implicit | |||
| 83 | + | extension bundles. `--no-ext --ext PATH` loads only the explicitly named bundle. | |||
| 84 | + | `--no-skill` disables all skills, including extension and built-in skills. | |||
| 81 | 85 | ||||
| 82 | When `XDG_STATE_HOME` is set on any platform, `$ZOT_HOME` defaults to | 86 | When `XDG_STATE_HOME` is set on any platform, `$ZOT_HOME` defaults to | ||
| 83 | `$XDG_STATE_HOME/zot`. Otherwise it defaults to `~/Library/Application Support/zot/` | 87 | `$XDG_STATE_HOME/zot`. Otherwise it defaults to `~/Library/Application Support/zot/` | ||
M packages/agent/skills/skills.go +97 -31
| @@ -21,14 +21,22 @@ | |||
| 21 | 21 | // | |
| 22 | 22 | // The compat paths are deliberate: a SKILL.md written for any of | |
| 23 | 23 | // the related ecosystems works in zot unchanged. | |
| 24 | + | // | |
| 25 | + | // Discovery walks each source recursively and follows symlinked | |
| 26 | + | // directories, so a skill can be a link into a shared checkout. A link | |
| 27 | + | // cycle is ignored, and each resolved directory is walked only once. | |
| 24 | 28 | package skills | |
| 25 | 29 | ||
| 26 | 30 | import ( | |
| 31 | + | "errors" | |
| 27 | 32 | "fmt" | |
| 33 | + | "io/fs" | |
| 28 | 34 | "os" | |
| 29 | 35 | "path/filepath" | |
| 36 | + | "runtime" | |
| 30 | 37 | "sort" | |
| 31 | 38 | "strings" | |
| 39 | + | "syscall" | |
| 32 | 40 | ) | |
| 33 | 41 | ||
| 34 | 42 | // Skill is one discovered SKILL.md file. | |
| @@ -207,50 +215,108 @@ func scanSource(source Source) ([]*Skill, []error) { | |||
| 207 | 215 | if err != nil { | |
| 208 | 216 | return out, []error{fmt.Errorf("resolve %s: %w", source.Root, err)} | |
| 209 | 217 | } | |
| 210 | - | err = filepath.WalkDir(root, func(path string, d os.DirEntry, walkErr error) error { | |
| 211 | - | if walkErr != nil { | |
| 212 | - | errs = append(errs, fmt.Errorf("%s: %w", path, walkErr)) | |
| 213 | - | return nil | |
| 218 | + | // filepath.WalkDir does not descend into symlinked directories, which | |
| 219 | + | // would silently drop a skill directory that links into a shared | |
| 220 | + | // checkout. Walk real directories before following links so an alias | |
| 221 | + | // cannot rename an existing unnamed skill. Record resolved directories | |
| 222 | + | // so a link cycle cannot recurse forever. | |
| 223 | + | visited := map[string]bool{} | |
| 224 | + | var linkedDirs []string | |
| 225 | + | var walk func(dir string) | |
| 226 | + | walk = func(dir string) { | |
| 227 | + | resolved, err := filepath.EvalSymlinks(dir) | |
| 228 | + | if err != nil { | |
| 229 | + | errs = append(errs, fmt.Errorf("resolve %s: %w", dir, err)) | |
| 230 | + | return | |
| 214 | 231 | } | |
| 215 | - | if d.IsDir() || d.Name() != "SKILL.md" { | |
| 216 | - | return nil | |
| 232 | + | if visited[resolved] { | |
| 233 | + | return | |
| 217 | 234 | } | |
| 218 | - | s, e := load(path, source.Label) | |
| 219 | - | if e != nil { | |
| 220 | - | errs = append(errs, fmt.Errorf("%s: %w", path, e)) | |
| 221 | - | return nil | |
| 235 | + | visited[resolved] = true | |
| 236 | + | entries, err := os.ReadDir(dir) | |
| 237 | + | if err != nil { | |
| 238 | + | errs = append(errs, fmt.Errorf("%s: %w", dir, err)) | |
| 239 | + | return | |
| 222 | 240 | } | |
| 223 | - | if s.Name == "" { | |
| 224 | - | rel, e := filepath.Rel(root, filepath.Dir(path)) | |
| 225 | - | if e != nil { | |
| 226 | - | errs = append(errs, e) | |
| 227 | - | return nil | |
| 241 | + | for _, d := range entries { | |
| 242 | + | path := filepath.Join(dir, d.Name()) | |
| 243 | + | if d.IsDir() { | |
| 244 | + | walk(path) | |
| 245 | + | continue | |
| 228 | 246 | } | |
| 229 | - | parts := []string{} | |
| 230 | - | if rel != "." { | |
| 231 | - | for _, p := range strings.Split(filepath.ToSlash(rel), "/") { | |
| 232 | - | if k := kebabCase(p); k != "" { | |
| 233 | - | parts = append(parts, k) | |
| 247 | + | if d.Type()&fs.ModeSymlink != 0 { | |
| 248 | + | info, statErr := os.Stat(path) // resolves the link | |
| 249 | + | if statErr != nil { | |
| 250 | + | // Dangling and cyclic links are not skills. Anything | |
| 251 | + | // else (permissions, I/O) is worth reporting. | |
| 252 | + | if !errors.Is(statErr, fs.ErrNotExist) && !symlinkLoopError(statErr) { | |
| 253 | + | errs = append(errs, fmt.Errorf("%s: %w", path, statErr)) | |
| 234 | 254 | } | |
| 255 | + | continue | |
| 256 | + | } | |
| 257 | + | if info.IsDir() { | |
| 258 | + | linkedDirs = append(linkedDirs, path) | |
| 259 | + | continue | |
| 235 | 260 | } | |
| 236 | 261 | } | |
| 237 | - | if len(parts) == 0 { | |
| 238 | - | parts = append(parts, kebabCase(filepath.Base(filepath.Dir(path)))) | |
| 262 | + | if d.Name() != "SKILL.md" { | |
| 263 | + | continue | |
| 239 | 264 | } | |
| 240 | - | s.Name = strings.Join(parts, "-") | |
| 241 | - | } | |
| 242 | - | if source.Prefix != "" { | |
| 243 | - | s.Name = source.Prefix + s.Name | |
| 265 | + | s, e := load(path, source.Label) | |
| 266 | + | if e != nil { | |
| 267 | + | errs = append(errs, fmt.Errorf("%s: %w", path, e)) | |
| 268 | + | continue | |
| 269 | + | } | |
| 270 | + | if s.Name == "" { | |
| 271 | + | name, e := deriveName(root, path) | |
| 272 | + | if e != nil { | |
| 273 | + | errs = append(errs, e) | |
| 274 | + | continue | |
| 275 | + | } | |
| 276 | + | s.Name = name | |
| 277 | + | } | |
| 278 | + | if source.Prefix != "" { | |
| 279 | + | s.Name = source.Prefix + s.Name | |
| 280 | + | } | |
| 281 | + | out = append(out, s) | |
| 244 | 282 | } | |
| 245 | - | out = append(out, s) | |
| 246 | - | return nil | |
| 247 | - | }) | |
| 248 | - | if err != nil { | |
| 249 | - | errs = append(errs, fmt.Errorf("scan %s: %w", root, err)) | |
| 283 | + | } | |
| 284 | + | walk(root) | |
| 285 | + | for i := 0; i < len(linkedDirs); i++ { | |
| 286 | + | walk(linkedDirs[i]) | |
| 250 | 287 | } | |
| 251 | 288 | return out, errs | |
| 252 | 289 | } | |
| 253 | 290 | ||
| 291 | + | func symlinkLoopError(err error) bool { | |
| 292 | + | // Windows reports cyclic links as ERROR_CANT_RESOLVE_FILENAME (1921), | |
| 293 | + | // rather than ELOOP. syscall does not export a name for this error. | |
| 294 | + | return errors.Is(err, syscall.ELOOP) || | |
| 295 | + | (runtime.GOOS == "windows" && errors.Is(err, syscall.Errno(1921))) | |
| 296 | + | } | |
| 297 | + | ||
| 298 | + | // deriveName builds a skill name for a SKILL.md file that carries no | |
| 299 | + | // frontmatter `name`, from its directory path relative to the source root. | |
| 300 | + | func deriveName(root, path string) (string, error) { | |
| 301 | + | dir := filepath.Dir(path) | |
| 302 | + | rel, err := filepath.Rel(root, dir) | |
| 303 | + | if err != nil { | |
| 304 | + | return "", fmt.Errorf("%s: %w", path, err) | |
| 305 | + | } | |
| 306 | + | var parts []string | |
| 307 | + | if rel != "." { | |
| 308 | + | for _, p := range strings.Split(filepath.ToSlash(rel), "/") { | |
| 309 | + | if k := kebabCase(p); k != "" { | |
| 310 | + | parts = append(parts, k) | |
| 311 | + | } | |
| 312 | + | } | |
| 313 | + | } | |
| 314 | + | if len(parts) == 0 { | |
| 315 | + | parts = append(parts, kebabCase(filepath.Base(dir))) | |
| 316 | + | } | |
| 317 | + | return strings.Join(parts, "-"), nil | |
| 318 | + | } | |
| 319 | + | ||
| 254 | 320 | // KebabCase converts a path component or display name to a stable skill name. | |
| 255 | 321 | func KebabCase(s string) string { return kebabCase(s) } | |
| 256 | 322 | ||
| @@ -21,14 +21,22 @@ | |||||
| 21 | // | 21 | // | ||
| 22 | // The compat paths are deliberate: a SKILL.md written for any of | 22 | // The compat paths are deliberate: a SKILL.md written for any of | ||
| 23 | // the related ecosystems works in zot unchanged. | 23 | // the related ecosystems works in zot unchanged. | ||
| 24 | + | // | |||
| 25 | + | // Discovery walks each source recursively and follows symlinked | |||
| 26 | + | // directories, so a skill can be a link into a shared checkout. A link | |||
| 27 | + | // cycle is ignored, and each resolved directory is walked only once. | |||
| 24 | package skills | 28 | package skills | ||
| 25 | 29 | ||||
| 26 | import ( | 30 | import ( | ||
| 31 | + | "errors" | |||
| 27 | "fmt" | 32 | "fmt" | ||
| 33 | + | "io/fs" | |||
| 28 | "os" | 34 | "os" | ||
| 29 | "path/filepath" | 35 | "path/filepath" | ||
| 36 | + | "runtime" | |||
| 30 | "sort" | 37 | "sort" | ||
| 31 | "strings" | 38 | "strings" | ||
| 39 | + | "syscall" | |||
| 32 | ) | 40 | ) | ||
| 33 | 41 | ||||
| 34 | // Skill is one discovered SKILL.md file. | 42 | // Skill is one discovered SKILL.md file. | ||
| @@ -207,50 +215,108 @@ func scanSource(source Source) ([]*Skill, []error) { | |||||
| 207 | if err != nil { | 215 | if err != nil { | ||
| 208 | return out, []error{fmt.Errorf("resolve %s: %w", source.Root, err)} | 216 | return out, []error{fmt.Errorf("resolve %s: %w", source.Root, err)} | ||
| 209 | } | 217 | } | ||
| 210 | - | err = filepath.WalkDir(root, func(path string, d os.DirEntry, walkErr error) error { | 218 | + | // filepath.WalkDir does not descend into symlinked directories, which |
| 211 | - | if walkErr != nil { | 219 | + | // would silently drop a skill directory that links into a shared |
| 212 | - | errs = append(errs, fmt.Errorf("%s: %w", path, walkErr)) | 220 | + | // checkout. Walk real directories before following links so an alias |
| 213 | - | return nil | 221 | + | // cannot rename an existing unnamed skill. Record resolved directories |
| 222 | + | // so a link cycle cannot recurse forever. | |||
| 223 | + | visited := map[string]bool{} | |||
| 224 | + | var linkedDirs []string | |||
| 225 | + | var walk func(dir string) | |||
| 226 | + | walk = func(dir string) { | |||
| 227 | + | resolved, err := filepath.EvalSymlinks(dir) | |||
| 228 | + | if err != nil { | |||
| 229 | + | errs = append(errs, fmt.Errorf("resolve %s: %w", dir, err)) | |||
| 230 | + | return | |||
| 214 | } | 231 | } | ||
| 215 | - | if d.IsDir() || d.Name() != "SKILL.md" { | 232 | + | if visited[resolved] { |
| 216 | - | return nil | 233 | + | return |
| 217 | } | 234 | } | ||
| 218 | - | s, e := load(path, source.Label) | 235 | + | visited[resolved] = true |
| 219 | - | if e != nil { | 236 | + | entries, err := os.ReadDir(dir) |
| 220 | - | errs = append(errs, fmt.Errorf("%s: %w", path, e)) | 237 | + | if err != nil { |
| 221 | - | return nil | 238 | + | errs = append(errs, fmt.Errorf("%s: %w", dir, err)) |
| 239 | + | return | |||
| 222 | } | 240 | } | ||
| 223 | - | if s.Name == "" { | 241 | + | for _, d := range entries { |
| 224 | - | rel, e := filepath.Rel(root, filepath.Dir(path)) | 242 | + | path := filepath.Join(dir, d.Name()) |
| 225 | - | if e != nil { | 243 | + | if d.IsDir() { |
| 226 | - | errs = append(errs, e) | 244 | + | walk(path) |
| 227 | - | return nil | 245 | + | continue |
| 228 | } | 246 | } | ||
| 229 | - | parts := []string{} | 247 | + | if d.Type()&fs.ModeSymlink != 0 { |
| 230 | - | if rel != "." { | 248 | + | info, statErr := os.Stat(path) // resolves the link |
| 231 | - | for _, p := range strings.Split(filepath.ToSlash(rel), "/") { | 249 | + | if statErr != nil { |
| 232 | - | if k := kebabCase(p); k != "" { | 250 | + | // Dangling and cyclic links are not skills. Anything |
| 233 | - | parts = append(parts, k) | 251 | + | // else (permissions, I/O) is worth reporting. |
| 252 | + | if !errors.Is(statErr, fs.ErrNotExist) && !symlinkLoopError(statErr) { | |||
| 253 | + | errs = append(errs, fmt.Errorf("%s: %w", path, statErr)) | |||
| 234 | } | 254 | } | ||
| 255 | + | continue | |||
| 256 | + | } | |||
| 257 | + | if info.IsDir() { | |||
| 258 | + | linkedDirs = append(linkedDirs, path) | |||
| 259 | + | continue | |||
| 235 | } | 260 | } | ||
| 236 | } | 261 | } | ||
| 237 | - | if len(parts) == 0 { | 262 | + | if d.Name() != "SKILL.md" { |
| 238 | - | parts = append(parts, kebabCase(filepath.Base(filepath.Dir(path)))) | 263 | + | continue |
| 239 | } | 264 | } | ||
| 240 | - | s.Name = strings.Join(parts, "-") | 265 | + | s, e := load(path, source.Label) |
| 241 | - | } | 266 | + | if e != nil { |
| 242 | - | if source.Prefix != "" { | 267 | + | errs = append(errs, fmt.Errorf("%s: %w", path, e)) |
| 243 | - | s.Name = source.Prefix + s.Name | 268 | + | continue |
| 269 | + | } | |||
| 270 | + | if s.Name == "" { | |||
| 271 | + | name, e := deriveName(root, path) | |||
| 272 | + | if e != nil { | |||
| 273 | + | errs = append(errs, e) | |||
| 274 | + | continue | |||
| 275 | + | } | |||
| 276 | + | s.Name = name | |||
| 277 | + | } | |||
| 278 | + | if source.Prefix != "" { | |||
| 279 | + | s.Name = source.Prefix + s.Name | |||
| 280 | + | } | |||
| 281 | + | out = append(out, s) | |||
| 244 | } | 282 | } | ||
| 245 | - | out = append(out, s) | 283 | + | } |
| 246 | - | return nil | 284 | + | walk(root) |
| 247 | - | }) | 285 | + | for i := 0; i < len(linkedDirs); i++ { |
| 248 | - | if err != nil { | 286 | + | walk(linkedDirs[i]) |
| 249 | - | errs = append(errs, fmt.Errorf("scan %s: %w", root, err)) | |||
| 250 | } | 287 | } | ||
| 251 | return out, errs | 288 | return out, errs | ||
| 252 | } | 289 | } | ||
| 253 | 290 | ||||
| 291 | + | func symlinkLoopError(err error) bool { | |||
| 292 | + | // Windows reports cyclic links as ERROR_CANT_RESOLVE_FILENAME (1921), | |||
| 293 | + | // rather than ELOOP. syscall does not export a name for this error. | |||
| 294 | + | return errors.Is(err, syscall.ELOOP) || | |||
| 295 | + | (runtime.GOOS == "windows" && errors.Is(err, syscall.Errno(1921))) | |||
| 296 | + | } | |||
| 297 | + | ||||
| 298 | + | // deriveName builds a skill name for a SKILL.md file that carries no | |||
| 299 | + | // frontmatter `name`, from its directory path relative to the source root. | |||
| 300 | + | func deriveName(root, path string) (string, error) { | |||
| 301 | + | dir := filepath.Dir(path) | |||
| 302 | + | rel, err := filepath.Rel(root, dir) | |||
| 303 | + | if err != nil { | |||
| 304 | + | return "", fmt.Errorf("%s: %w", path, err) | |||
| 305 | + | } | |||
| 306 | + | var parts []string | |||
| 307 | + | if rel != "." { | |||
| 308 | + | for _, p := range strings.Split(filepath.ToSlash(rel), "/") { | |||
| 309 | + | if k := kebabCase(p); k != "" { | |||
| 310 | + | parts = append(parts, k) | |||
| 311 | + | } | |||
| 312 | + | } | |||
| 313 | + | } | |||
| 314 | + | if len(parts) == 0 { | |||
| 315 | + | parts = append(parts, kebabCase(filepath.Base(dir))) | |||
| 316 | + | } | |||
| 317 | + | return strings.Join(parts, "-"), nil | |||
| 318 | + | } | |||
| 319 | + | ||||
| 254 | // KebabCase converts a path component or display name to a stable skill name. | 320 | // KebabCase converts a path component or display name to a stable skill name. | ||
| 255 | func KebabCase(s string) string { return kebabCase(s) } | 321 | func KebabCase(s string) string { return kebabCase(s) } | ||
| 256 | 322 | ||||
M packages/agent/skills/skills_test.go +146 -0
| @@ -1,10 +1,12 @@ | |||
| 1 | 1 | package skills | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "fmt" | |
| 4 | 5 | "os" | |
| 5 | 6 | "path/filepath" | |
| 6 | 7 | "runtime" | |
| 7 | 8 | "strings" | |
| 9 | + | "syscall" | |
| 8 | 10 | "testing" | |
| 9 | 11 | ) | |
| 10 | 12 | ||
| @@ -147,6 +149,150 @@ func TestDiscoverSourcesFollowsSymlinkedRoot(t *testing.T) { | |||
| 147 | 149 | } | |
| 148 | 150 | } | |
| 149 | 151 | ||
| 152 | + | func TestDiscoverSourcesFollowsSymlinkedSkillDir(t *testing.T) { | |
| 153 | + | shared := t.TempDir() | |
| 154 | + | writeSkill(t, filepath.Join(shared, "tdd"), "name: tdd", "shared body") | |
| 155 | + | root := t.TempDir() | |
| 156 | + | linkDir(t, filepath.Join(shared, "tdd"), filepath.Join(root, "tdd")) | |
| 157 | + | ||
| 158 | + | got, errs := DiscoverSources([]Source{{Root: root, Label: "global"}}, false) | |
| 159 | + | if len(errs) != 0 || len(got) != 1 { | |
| 160 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |
| 161 | + | } | |
| 162 | + | if got[0].Name != "tdd" || got[0].Body != "shared body" || got[0].Source != "global" { | |
| 163 | + | t.Errorf("skill = %#v", got[0]) | |
| 164 | + | } | |
| 165 | + | if want := filepath.Join(mustEvalSymlinks(t, root), "tdd", "SKILL.md"); got[0].Path != want { | |
| 166 | + | t.Errorf("path = %q, want the link location %q", got[0].Path, want) | |
| 167 | + | } | |
| 168 | + | } | |
| 169 | + | ||
| 170 | + | func TestDiscoverSourcesFollowsSymlinkedCategoryDir(t *testing.T) { | |
| 171 | + | shared := t.TempDir() | |
| 172 | + | writeSkill(t, filepath.Join(shared, "code-review"), "name: code-review", "named body") | |
| 173 | + | writeSkill(t, filepath.Join(shared, "prototype"), "", "unnamed body") | |
| 174 | + | root := t.TempDir() | |
| 175 | + | linkDir(t, shared, filepath.Join(root, "engineering")) | |
| 176 | + | ||
| 177 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |
| 178 | + | if len(errs) != 0 || len(got) != 2 { | |
| 179 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |
| 180 | + | } | |
| 181 | + | if got[0].Name != "code-review" || got[1].Name != "engineering-prototype" { | |
| 182 | + | t.Errorf("names = %q, %q", got[0].Name, got[1].Name) | |
| 183 | + | } | |
| 184 | + | } | |
| 185 | + | ||
| 186 | + | func TestDiscoverSourcesSymlinkCycleTerminates(t *testing.T) { | |
| 187 | + | root := t.TempDir() | |
| 188 | + | writeSkill(t, filepath.Join(root, "review"), "name: review", "body") | |
| 189 | + | linkDir(t, root, filepath.Join(root, "loop")) | |
| 190 | + | linkDir(t, root, filepath.Join(root, "alias")) | |
| 191 | + | ||
| 192 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |
| 193 | + | if len(errs) != 0 || len(got) != 1 || got[0].Name != "review" { | |
| 194 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |
| 195 | + | } | |
| 196 | + | } | |
| 197 | + | ||
| 198 | + | func TestDiscoverSourcesIgnoresDanglingSymlink(t *testing.T) { | |
| 199 | + | root := t.TempDir() | |
| 200 | + | writeSkill(t, filepath.Join(root, "review"), "name: review", "body") | |
| 201 | + | missing := filepath.Join(t.TempDir(), "gone") | |
| 202 | + | linkDir(t, missing, filepath.Join(root, "gone")) | |
| 203 | + | ||
| 204 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |
| 205 | + | if len(errs) != 0 || len(got) != 1 || got[0].Name != "review" { | |
| 206 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |
| 207 | + | } | |
| 208 | + | } | |
| 209 | + | ||
| 210 | + | func TestDiscoverSourcesWalksLinkedDirOnce(t *testing.T) { | |
| 211 | + | root := t.TempDir() | |
| 212 | + | real := filepath.Join(root, "shared") | |
| 213 | + | writeSkill(t, real, "name: review", "body") | |
| 214 | + | linkDir(t, real, filepath.Join(root, "alias")) | |
| 215 | + | ||
| 216 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |
| 217 | + | if len(errs) != 0 || len(got) != 1 { | |
| 218 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |
| 219 | + | } | |
| 220 | + | if want := filepath.Join(mustEvalSymlinks(t, root), "shared", "SKILL.md"); got[0].Path != want { | |
| 221 | + | t.Errorf("path = %q, want %q", got[0].Path, want) | |
| 222 | + | } | |
| 223 | + | } | |
| 224 | + | ||
| 225 | + | func TestDiscoverSourcesKeepsUnnamedRealSkillWhenAliasSortsFirst(t *testing.T) { | |
| 226 | + | root := t.TempDir() | |
| 227 | + | real := filepath.Join(root, "z", "review") | |
| 228 | + | writeSkill(t, real, "", "body") | |
| 229 | + | linkDir(t, real, filepath.Join(root, "alias")) | |
| 230 | + | ||
| 231 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |
| 232 | + | if len(errs) != 0 || len(got) != 1 { | |
| 233 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |
| 234 | + | } | |
| 235 | + | if got[0].Name != "z-review" || got[0].Path != filepath.Join(mustEvalSymlinks(t, root), "z", "review", "SKILL.md") { | |
| 236 | + | t.Errorf("skill = %#v, want the real directory name and path", got[0]) | |
| 237 | + | } | |
| 238 | + | } | |
| 239 | + | ||
| 240 | + | func TestDiscoverSourcesIgnoresCyclicSymlink(t *testing.T) { | |
| 241 | + | root := t.TempDir() | |
| 242 | + | writeSkill(t, filepath.Join(root, "review"), "name: review", "body") | |
| 243 | + | linkDir(t, filepath.Join(root, "loop"), filepath.Join(root, "loop")) | |
| 244 | + | ||
| 245 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |
| 246 | + | if len(errs) != 0 || len(got) != 1 || got[0].Name != "review" { | |
| 247 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |
| 248 | + | } | |
| 249 | + | } | |
| 250 | + | ||
| 251 | + | func TestSymlinkLoopError(t *testing.T) { | |
| 252 | + | if !symlinkLoopError(fmt.Errorf("stat: %w", syscall.ELOOP)) { | |
| 253 | + | t.Error("ELOOP should be ignored") | |
| 254 | + | } | |
| 255 | + | if runtime.GOOS == "windows" && !symlinkLoopError(fmt.Errorf("stat: %w", syscall.Errno(1921))) { | |
| 256 | + | t.Error("Windows cyclic-link error should be ignored") | |
| 257 | + | } | |
| 258 | + | if symlinkLoopError(fmt.Errorf("stat: %w", os.ErrPermission)) { | |
| 259 | + | t.Error("permission errors must still be reported") | |
| 260 | + | } | |
| 261 | + | } | |
| 262 | + | ||
| 263 | + | func mustEvalSymlinks(t *testing.T, path string) string { | |
| 264 | + | t.Helper() | |
| 265 | + | resolved, err := filepath.EvalSymlinks(path) | |
| 266 | + | if err != nil { | |
| 267 | + | t.Fatal(err) | |
| 268 | + | } | |
| 269 | + | return resolved | |
| 270 | + | } | |
| 271 | + | ||
| 272 | + | func writeSkill(t *testing.T, dir, front, body string) { | |
| 273 | + | t.Helper() | |
| 274 | + | if err := os.MkdirAll(dir, 0o755); err != nil { | |
| 275 | + | t.Fatal(err) | |
| 276 | + | } | |
| 277 | + | frontmatter := "---\ndescription: test\n---\n" | |
| 278 | + | if front != "" { | |
| 279 | + | frontmatter = "---\n" + front + "\ndescription: test\n---\n" | |
| 280 | + | } | |
| 281 | + | if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(frontmatter+body), 0o644); err != nil { | |
| 282 | + | t.Fatal(err) | |
| 283 | + | } | |
| 284 | + | } | |
| 285 | + | ||
| 286 | + | func linkDir(t *testing.T, target, path string) { | |
| 287 | + | t.Helper() | |
| 288 | + | if err := os.Symlink(target, path); err != nil { | |
| 289 | + | if runtime.GOOS == "windows" { | |
| 290 | + | t.Skipf("symlinks unavailable: %v", err) | |
| 291 | + | } | |
| 292 | + | t.Fatal(err) | |
| 293 | + | } | |
| 294 | + | } | |
| 295 | + | ||
| 150 | 296 | func TestDiscoverSourcesRecursiveAndPrefixed(t *testing.T) { | |
| 151 | 297 | root := t.TempDir() | |
| 152 | 298 | path := filepath.Join(root, "Something", "Writing Git Commits") | |
| @@ -1,10 +1,12 @@ | |||||
| 1 | package skills | 1 | package skills | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "fmt" | |||
| 4 | "os" | 5 | "os" | ||
| 5 | "path/filepath" | 6 | "path/filepath" | ||
| 6 | "runtime" | 7 | "runtime" | ||
| 7 | "strings" | 8 | "strings" | ||
| 9 | + | "syscall" | |||
| 8 | "testing" | 10 | "testing" | ||
| 9 | ) | 11 | ) | ||
| 10 | 12 | ||||
| @@ -147,6 +149,150 @@ func TestDiscoverSourcesFollowsSymlinkedRoot(t *testing.T) { | |||||
| 147 | } | 149 | } | ||
| 148 | } | 150 | } | ||
| 149 | 151 | ||||
| 152 | + | func TestDiscoverSourcesFollowsSymlinkedSkillDir(t *testing.T) { | |||
| 153 | + | shared := t.TempDir() | |||
| 154 | + | writeSkill(t, filepath.Join(shared, "tdd"), "name: tdd", "shared body") | |||
| 155 | + | root := t.TempDir() | |||
| 156 | + | linkDir(t, filepath.Join(shared, "tdd"), filepath.Join(root, "tdd")) | |||
| 157 | + | ||||
| 158 | + | got, errs := DiscoverSources([]Source{{Root: root, Label: "global"}}, false) | |||
| 159 | + | if len(errs) != 0 || len(got) != 1 { | |||
| 160 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |||
| 161 | + | } | |||
| 162 | + | if got[0].Name != "tdd" || got[0].Body != "shared body" || got[0].Source != "global" { | |||
| 163 | + | t.Errorf("skill = %#v", got[0]) | |||
| 164 | + | } | |||
| 165 | + | if want := filepath.Join(mustEvalSymlinks(t, root), "tdd", "SKILL.md"); got[0].Path != want { | |||
| 166 | + | t.Errorf("path = %q, want the link location %q", got[0].Path, want) | |||
| 167 | + | } | |||
| 168 | + | } | |||
| 169 | + | ||||
| 170 | + | func TestDiscoverSourcesFollowsSymlinkedCategoryDir(t *testing.T) { | |||
| 171 | + | shared := t.TempDir() | |||
| 172 | + | writeSkill(t, filepath.Join(shared, "code-review"), "name: code-review", "named body") | |||
| 173 | + | writeSkill(t, filepath.Join(shared, "prototype"), "", "unnamed body") | |||
| 174 | + | root := t.TempDir() | |||
| 175 | + | linkDir(t, shared, filepath.Join(root, "engineering")) | |||
| 176 | + | ||||
| 177 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |||
| 178 | + | if len(errs) != 0 || len(got) != 2 { | |||
| 179 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |||
| 180 | + | } | |||
| 181 | + | if got[0].Name != "code-review" || got[1].Name != "engineering-prototype" { | |||
| 182 | + | t.Errorf("names = %q, %q", got[0].Name, got[1].Name) | |||
| 183 | + | } | |||
| 184 | + | } | |||
| 185 | + | ||||
| 186 | + | func TestDiscoverSourcesSymlinkCycleTerminates(t *testing.T) { | |||
| 187 | + | root := t.TempDir() | |||
| 188 | + | writeSkill(t, filepath.Join(root, "review"), "name: review", "body") | |||
| 189 | + | linkDir(t, root, filepath.Join(root, "loop")) | |||
| 190 | + | linkDir(t, root, filepath.Join(root, "alias")) | |||
| 191 | + | ||||
| 192 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |||
| 193 | + | if len(errs) != 0 || len(got) != 1 || got[0].Name != "review" { | |||
| 194 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |||
| 195 | + | } | |||
| 196 | + | } | |||
| 197 | + | ||||
| 198 | + | func TestDiscoverSourcesIgnoresDanglingSymlink(t *testing.T) { | |||
| 199 | + | root := t.TempDir() | |||
| 200 | + | writeSkill(t, filepath.Join(root, "review"), "name: review", "body") | |||
| 201 | + | missing := filepath.Join(t.TempDir(), "gone") | |||
| 202 | + | linkDir(t, missing, filepath.Join(root, "gone")) | |||
| 203 | + | ||||
| 204 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |||
| 205 | + | if len(errs) != 0 || len(got) != 1 || got[0].Name != "review" { | |||
| 206 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |||
| 207 | + | } | |||
| 208 | + | } | |||
| 209 | + | ||||
| 210 | + | func TestDiscoverSourcesWalksLinkedDirOnce(t *testing.T) { | |||
| 211 | + | root := t.TempDir() | |||
| 212 | + | real := filepath.Join(root, "shared") | |||
| 213 | + | writeSkill(t, real, "name: review", "body") | |||
| 214 | + | linkDir(t, real, filepath.Join(root, "alias")) | |||
| 215 | + | ||||
| 216 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |||
| 217 | + | if len(errs) != 0 || len(got) != 1 { | |||
| 218 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |||
| 219 | + | } | |||
| 220 | + | if want := filepath.Join(mustEvalSymlinks(t, root), "shared", "SKILL.md"); got[0].Path != want { | |||
| 221 | + | t.Errorf("path = %q, want %q", got[0].Path, want) | |||
| 222 | + | } | |||
| 223 | + | } | |||
| 224 | + | ||||
| 225 | + | func TestDiscoverSourcesKeepsUnnamedRealSkillWhenAliasSortsFirst(t *testing.T) { | |||
| 226 | + | root := t.TempDir() | |||
| 227 | + | real := filepath.Join(root, "z", "review") | |||
| 228 | + | writeSkill(t, real, "", "body") | |||
| 229 | + | linkDir(t, real, filepath.Join(root, "alias")) | |||
| 230 | + | ||||
| 231 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |||
| 232 | + | if len(errs) != 0 || len(got) != 1 { | |||
| 233 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |||
| 234 | + | } | |||
| 235 | + | if got[0].Name != "z-review" || got[0].Path != filepath.Join(mustEvalSymlinks(t, root), "z", "review", "SKILL.md") { | |||
| 236 | + | t.Errorf("skill = %#v, want the real directory name and path", got[0]) | |||
| 237 | + | } | |||
| 238 | + | } | |||
| 239 | + | ||||
| 240 | + | func TestDiscoverSourcesIgnoresCyclicSymlink(t *testing.T) { | |||
| 241 | + | root := t.TempDir() | |||
| 242 | + | writeSkill(t, filepath.Join(root, "review"), "name: review", "body") | |||
| 243 | + | linkDir(t, filepath.Join(root, "loop"), filepath.Join(root, "loop")) | |||
| 244 | + | ||||
| 245 | + | got, errs := DiscoverSources([]Source{{Root: root}}, false) | |||
| 246 | + | if len(errs) != 0 || len(got) != 1 || got[0].Name != "review" { | |||
| 247 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |||
| 248 | + | } | |||
| 249 | + | } | |||
| 250 | + | ||||
| 251 | + | func TestSymlinkLoopError(t *testing.T) { | |||
| 252 | + | if !symlinkLoopError(fmt.Errorf("stat: %w", syscall.ELOOP)) { | |||
| 253 | + | t.Error("ELOOP should be ignored") | |||
| 254 | + | } | |||
| 255 | + | if runtime.GOOS == "windows" && !symlinkLoopError(fmt.Errorf("stat: %w", syscall.Errno(1921))) { | |||
| 256 | + | t.Error("Windows cyclic-link error should be ignored") | |||
| 257 | + | } | |||
| 258 | + | if symlinkLoopError(fmt.Errorf("stat: %w", os.ErrPermission)) { | |||
| 259 | + | t.Error("permission errors must still be reported") | |||
| 260 | + | } | |||
| 261 | + | } | |||
| 262 | + | ||||
| 263 | + | func mustEvalSymlinks(t *testing.T, path string) string { | |||
| 264 | + | t.Helper() | |||
| 265 | + | resolved, err := filepath.EvalSymlinks(path) | |||
| 266 | + | if err != nil { | |||
| 267 | + | t.Fatal(err) | |||
| 268 | + | } | |||
| 269 | + | return resolved | |||
| 270 | + | } | |||
| 271 | + | ||||
| 272 | + | func writeSkill(t *testing.T, dir, front, body string) { | |||
| 273 | + | t.Helper() | |||
| 274 | + | if err := os.MkdirAll(dir, 0o755); err != nil { | |||
| 275 | + | t.Fatal(err) | |||
| 276 | + | } | |||
| 277 | + | frontmatter := "---\ndescription: test\n---\n" | |||
| 278 | + | if front != "" { | |||
| 279 | + | frontmatter = "---\n" + front + "\ndescription: test\n---\n" | |||
| 280 | + | } | |||
| 281 | + | if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(frontmatter+body), 0o644); err != nil { | |||
| 282 | + | t.Fatal(err) | |||
| 283 | + | } | |||
| 284 | + | } | |||
| 285 | + | ||||
| 286 | + | func linkDir(t *testing.T, target, path string) { | |||
| 287 | + | t.Helper() | |||
| 288 | + | if err := os.Symlink(target, path); err != nil { | |||
| 289 | + | if runtime.GOOS == "windows" { | |||
| 290 | + | t.Skipf("symlinks unavailable: %v", err) | |||
| 291 | + | } | |||
| 292 | + | t.Fatal(err) | |||
| 293 | + | } | |||
| 294 | + | } | |||
| 295 | + | ||||
| 150 | func TestDiscoverSourcesRecursiveAndPrefixed(t *testing.T) { | 296 | func TestDiscoverSourcesRecursiveAndPrefixed(t *testing.T) { | ||
| 151 | root := t.TempDir() | 297 | root := t.TempDir() | ||
| 152 | path := filepath.Join(root, "Something", "Writing Git Commits") | 298 | path := filepath.Join(root, "Something", "Writing Git Commits") | ||