fix: suppress skill self-shadowing warnings
- commit
- 5f8ce64f751ffacc6a2a6671a2dc02ac07a74fdf
- author
- patriceckhart · · 2026-09-17 05:40 UTC
- parent
- 860c2f8
- signature
- unsigned
- refs
- tag: v0.3.77
3 files · +79 -2
M docs/skills.md +5 -2
| @@ -71,8 +71,11 @@ Only files named exactly `SKILL.md` are read. | |||
| 71 | 71 | ||
| 72 | 72 | Extensions namespace their skills, for example `git-tools:writing-git-commits`. | |
| 73 | 73 | Explicit `--ext` bundles take precedence over environment, project, global, and | |
| 74 | - | compatibility sources. Duplicate names keep the higher-precedence file and | |
| 75 | - | report a diagnostic. `--no-ext` omits implicit extension bundles; | |
| 74 | + | compatibility sources. Duplicate names from different files keep the | |
| 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. | |
| 77 | + | This includes overlapping project and global locations when running from your | |
| 78 | + | home directory. `--no-ext` omits implicit extension bundles; | |
| 76 | 79 | `--no-ext --ext PATH` loads only the explicitly named bundle. `--no-skill` | |
| 77 | 80 | disables all skills, including extension and built-in skills. | |
| 78 | 81 | ||
| @@ -71,8 +71,11 @@ Only files named exactly `SKILL.md` are read. | |||||
| 71 | 71 | ||||
| 72 | Extensions namespace their skills, for example `git-tools:writing-git-commits`. | 72 | Extensions namespace their skills, for example `git-tools:writing-git-commits`. | ||
| 73 | Explicit `--ext` bundles take precedence over environment, project, global, and | 73 | Explicit `--ext` bundles take precedence over environment, project, global, and | ||
| 74 | - | compatibility sources. Duplicate names keep the higher-precedence file and | 74 | + | compatibility sources. Duplicate names from different files keep the |
| 75 | - | report a diagnostic. `--no-ext` omits implicit extension bundles; | 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. | |||
| 77 | + | This includes overlapping project and global locations when running from your | |||
| 78 | + | home directory. `--no-ext` omits implicit extension bundles; | |||
| 76 | `--no-ext --ext PATH` loads only the explicitly named bundle. `--no-skill` | 79 | `--no-ext --ext PATH` loads only the explicitly named bundle. `--no-skill` | ||
| 77 | disables all skills, including extension and built-in skills. | 80 | disables all skills, including extension and built-in skills. | ||
| 78 | 81 | ||||
M packages/agent/skills/skills.go +14 -0
| @@ -140,6 +140,9 @@ func DiscoverSources(sources []Source, includeBuiltins bool) ([]*Skill, []error) | |||
| 140 | 140 | errs = append(errs, scanErrs...) | |
| 141 | 141 | for _, s := range found { | |
| 142 | 142 | if prior, dup := seen[s.Name]; dup { | |
| 143 | + | if sameSkillFile(prior.Path, s.Path) { | |
| 144 | + | continue | |
| 145 | + | } | |
| 143 | 146 | errs = append(errs, fmt.Errorf("skill %q shadowed: selected %s; ignored %s", s.Name, prior.Path, s.Path)) | |
| 144 | 147 | continue | |
| 145 | 148 | } | |
| @@ -163,6 +166,17 @@ func DiscoverSources(sources []Source, includeBuiltins bool) ([]*Skill, []error) | |||
| 163 | 166 | return out, errs | |
| 164 | 167 | } | |
| 165 | 168 | ||
| 169 | + | // sameSkillFile distinguishes repeated discovery from a genuine name conflict. | |
| 170 | + | // Stat follows symlinks, and SameFile also handles filesystem path aliases. | |
| 171 | + | func sameSkillFile(a, b string) bool { | |
| 172 | + | if a == b { | |
| 173 | + | return true | |
| 174 | + | } | |
| 175 | + | aInfo, aErr := os.Stat(a) | |
| 176 | + | bInfo, bErr := os.Stat(b) | |
| 177 | + | return aErr == nil && bErr == nil && os.SameFile(aInfo, bInfo) | |
| 178 | + | } | |
| 179 | + | ||
| 166 | 180 | func scanSource(source Source) ([]*Skill, []error) { | |
| 167 | 181 | var out []*Skill | |
| 168 | 182 | var errs []error | |
| @@ -140,6 +140,9 @@ func DiscoverSources(sources []Source, includeBuiltins bool) ([]*Skill, []error) | |||||
| 140 | errs = append(errs, scanErrs...) | 140 | errs = append(errs, scanErrs...) | ||
| 141 | for _, s := range found { | 141 | for _, s := range found { | ||
| 142 | if prior, dup := seen[s.Name]; dup { | 142 | if prior, dup := seen[s.Name]; dup { | ||
| 143 | + | if sameSkillFile(prior.Path, s.Path) { | |||
| 144 | + | continue | |||
| 145 | + | } | |||
| 143 | errs = append(errs, fmt.Errorf("skill %q shadowed: selected %s; ignored %s", s.Name, prior.Path, s.Path)) | 146 | errs = append(errs, fmt.Errorf("skill %q shadowed: selected %s; ignored %s", s.Name, prior.Path, s.Path)) | ||
| 144 | continue | 147 | continue | ||
| 145 | } | 148 | } | ||
| @@ -163,6 +166,17 @@ func DiscoverSources(sources []Source, includeBuiltins bool) ([]*Skill, []error) | |||||
| 163 | return out, errs | 166 | return out, errs | ||
| 164 | } | 167 | } | ||
| 165 | 168 | ||||
| 169 | + | // sameSkillFile distinguishes repeated discovery from a genuine name conflict. | |||
| 170 | + | // Stat follows symlinks, and SameFile also handles filesystem path aliases. | |||
| 171 | + | func sameSkillFile(a, b string) bool { | |||
| 172 | + | if a == b { | |||
| 173 | + | return true | |||
| 174 | + | } | |||
| 175 | + | aInfo, aErr := os.Stat(a) | |||
| 176 | + | bInfo, bErr := os.Stat(b) | |||
| 177 | + | return aErr == nil && bErr == nil && os.SameFile(aInfo, bInfo) | |||
| 178 | + | } | |||
| 179 | + | ||||
| 166 | func scanSource(source Source) ([]*Skill, []error) { | 180 | func scanSource(source Source) ([]*Skill, []error) { | ||
| 167 | var out []*Skill | 181 | var out []*Skill | ||
| 168 | var errs []error | 182 | var errs []error | ||
M packages/agent/skills/skills_test.go +60 -0
| @@ -169,6 +169,66 @@ func TestDiscoverSourcesRecursiveAndPrefixed(t *testing.T) { | |||
| 169 | 169 | } | |
| 170 | 170 | } | |
| 171 | 171 | ||
| 172 | + | func TestDiscoverSourcesHomeDirectoryOverlap(t *testing.T) { | |
| 173 | + | t.Setenv("ZOT_AGENT_SKILLS", "") | |
| 174 | + | home := t.TempDir() | |
| 175 | + | for _, compat := range []string{".agents", ".claude"} { | |
| 176 | + | dir := filepath.Join(home, compat, "skills", "review") | |
| 177 | + | if err := os.MkdirAll(dir, 0o755); err != nil { | |
| 178 | + | t.Fatal(err) | |
| 179 | + | } | |
| 180 | + | if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("---\nname: "+compat[1:]+"-review\n---\nbody"), 0o644); err != nil { | |
| 181 | + | t.Fatal(err) | |
| 182 | + | } | |
| 183 | + | } | |
| 184 | + | got, errs := DiscoverSources(SearchSources(t.TempDir(), home, home), false) | |
| 185 | + | if len(got) != 2 || len(errs) != 0 { | |
| 186 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |
| 187 | + | } | |
| 188 | + | for _, s := range got { | |
| 189 | + | if !strings.HasPrefix(s.Source, "project") { | |
| 190 | + | t.Errorf("source = %q, want project precedence", s.Source) | |
| 191 | + | } | |
| 192 | + | } | |
| 193 | + | } | |
| 194 | + | ||
| 195 | + | func TestDiscoverSourcesSameFile(t *testing.T) { | |
| 196 | + | root := t.TempDir() | |
| 197 | + | path := filepath.Join(root, "SKILL.md") | |
| 198 | + | if err := os.WriteFile(path, []byte("---\nname: review\n---\nbody"), 0o644); err != nil { | |
| 199 | + | t.Fatal(err) | |
| 200 | + | } | |
| 201 | + | for _, second := range []string{root, path} { | |
| 202 | + | got, errs := DiscoverSources([]Source{{Root: root, Label: "first"}, {Root: second, Label: "second"}}, false) | |
| 203 | + | if len(got) != 1 || len(errs) != 0 || got[0].Source != "first" { | |
| 204 | + | t.Fatalf("second=%q skills=%#v errs=%v", second, got, errs) | |
| 205 | + | } | |
| 206 | + | } | |
| 207 | + | got, errs := DiscoverSources([]Source{{Root: root, Prefix: "one:"}, {Root: root, Prefix: "two:"}}, false) | |
| 208 | + | if len(got) != 2 || len(errs) != 0 || got[0].Name != "one:review" || got[1].Name != "two:review" { | |
| 209 | + | t.Fatalf("namespaced skills=%#v errs=%v", got, errs) | |
| 210 | + | } | |
| 211 | + | } | |
| 212 | + | ||
| 213 | + | func TestDiscoverSourcesSameFileSymlink(t *testing.T) { | |
| 214 | + | root := t.TempDir() | |
| 215 | + | path := filepath.Join(root, "SKILL.md") | |
| 216 | + | if err := os.WriteFile(path, []byte("---\nname: review\n---\nbody"), 0o644); err != nil { | |
| 217 | + | t.Fatal(err) | |
| 218 | + | } | |
| 219 | + | alias := filepath.Join(t.TempDir(), "SKILL.md") | |
| 220 | + | if err := os.Symlink(path, alias); err != nil { | |
| 221 | + | if runtime.GOOS == "windows" { | |
| 222 | + | t.Skipf("symlinks unavailable: %v", err) | |
| 223 | + | } | |
| 224 | + | t.Fatal(err) | |
| 225 | + | } | |
| 226 | + | got, errs := DiscoverSources([]Source{{Root: root, Label: "first"}, {Root: alias}}, false) | |
| 227 | + | if len(got) != 1 || len(errs) != 0 || got[0].Source != "first" { | |
| 228 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |
| 229 | + | } | |
| 230 | + | } | |
| 231 | + | ||
| 172 | 232 | func TestDiscoverSourcesDuplicateDiagnostic(t *testing.T) { | |
| 173 | 233 | a, b := t.TempDir(), t.TempDir() | |
| 174 | 234 | for _, root := range []string{a, b} { | |
| @@ -169,6 +169,66 @@ func TestDiscoverSourcesRecursiveAndPrefixed(t *testing.T) { | |||||
| 169 | } | 169 | } | ||
| 170 | } | 170 | } | ||
| 171 | 171 | ||||
| 172 | + | func TestDiscoverSourcesHomeDirectoryOverlap(t *testing.T) { | |||
| 173 | + | t.Setenv("ZOT_AGENT_SKILLS", "") | |||
| 174 | + | home := t.TempDir() | |||
| 175 | + | for _, compat := range []string{".agents", ".claude"} { | |||
| 176 | + | dir := filepath.Join(home, compat, "skills", "review") | |||
| 177 | + | if err := os.MkdirAll(dir, 0o755); err != nil { | |||
| 178 | + | t.Fatal(err) | |||
| 179 | + | } | |||
| 180 | + | if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("---\nname: "+compat[1:]+"-review\n---\nbody"), 0o644); err != nil { | |||
| 181 | + | t.Fatal(err) | |||
| 182 | + | } | |||
| 183 | + | } | |||
| 184 | + | got, errs := DiscoverSources(SearchSources(t.TempDir(), home, home), false) | |||
| 185 | + | if len(got) != 2 || len(errs) != 0 { | |||
| 186 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |||
| 187 | + | } | |||
| 188 | + | for _, s := range got { | |||
| 189 | + | if !strings.HasPrefix(s.Source, "project") { | |||
| 190 | + | t.Errorf("source = %q, want project precedence", s.Source) | |||
| 191 | + | } | |||
| 192 | + | } | |||
| 193 | + | } | |||
| 194 | + | ||||
| 195 | + | func TestDiscoverSourcesSameFile(t *testing.T) { | |||
| 196 | + | root := t.TempDir() | |||
| 197 | + | path := filepath.Join(root, "SKILL.md") | |||
| 198 | + | if err := os.WriteFile(path, []byte("---\nname: review\n---\nbody"), 0o644); err != nil { | |||
| 199 | + | t.Fatal(err) | |||
| 200 | + | } | |||
| 201 | + | for _, second := range []string{root, path} { | |||
| 202 | + | got, errs := DiscoverSources([]Source{{Root: root, Label: "first"}, {Root: second, Label: "second"}}, false) | |||
| 203 | + | if len(got) != 1 || len(errs) != 0 || got[0].Source != "first" { | |||
| 204 | + | t.Fatalf("second=%q skills=%#v errs=%v", second, got, errs) | |||
| 205 | + | } | |||
| 206 | + | } | |||
| 207 | + | got, errs := DiscoverSources([]Source{{Root: root, Prefix: "one:"}, {Root: root, Prefix: "two:"}}, false) | |||
| 208 | + | if len(got) != 2 || len(errs) != 0 || got[0].Name != "one:review" || got[1].Name != "two:review" { | |||
| 209 | + | t.Fatalf("namespaced skills=%#v errs=%v", got, errs) | |||
| 210 | + | } | |||
| 211 | + | } | |||
| 212 | + | ||||
| 213 | + | func TestDiscoverSourcesSameFileSymlink(t *testing.T) { | |||
| 214 | + | root := t.TempDir() | |||
| 215 | + | path := filepath.Join(root, "SKILL.md") | |||
| 216 | + | if err := os.WriteFile(path, []byte("---\nname: review\n---\nbody"), 0o644); err != nil { | |||
| 217 | + | t.Fatal(err) | |||
| 218 | + | } | |||
| 219 | + | alias := filepath.Join(t.TempDir(), "SKILL.md") | |||
| 220 | + | if err := os.Symlink(path, alias); err != nil { | |||
| 221 | + | if runtime.GOOS == "windows" { | |||
| 222 | + | t.Skipf("symlinks unavailable: %v", err) | |||
| 223 | + | } | |||
| 224 | + | t.Fatal(err) | |||
| 225 | + | } | |||
| 226 | + | got, errs := DiscoverSources([]Source{{Root: root, Label: "first"}, {Root: alias}}, false) | |||
| 227 | + | if len(got) != 1 || len(errs) != 0 || got[0].Source != "first" { | |||
| 228 | + | t.Fatalf("skills=%#v errs=%v", got, errs) | |||
| 229 | + | } | |||
| 230 | + | } | |||
| 231 | + | ||||
| 172 | func TestDiscoverSourcesDuplicateDiagnostic(t *testing.T) { | 232 | func TestDiscoverSourcesDuplicateDiagnostic(t *testing.T) { | ||
| 173 | a, b := t.TempDir(), t.TempDir() | 233 | a, b := t.TempDir(), t.TempDir() | ||
| 174 | for _, root := range []string{a, b} { | 234 | for _, root := range []string{a, b} { | ||