93ad671 Fix extension skill discovery through symlinks · zut

M packages/agent/extensions/manager.go +32 -15


@@ -199,19 +199,17 @@ func (m *Manager) Discover(ctx context.Context) []error {
199	199		var jobs []loadJob
200	200		seenDirs := map[string]bool{} // dedup by basename so project wins
201	201		for _, dir := range m.searchDirs() {
202		-	entries, err := os.ReadDir(dir)
	202	+	extDirs, err := extensionDirs(dir)
203	203		if err != nil {
204	204		continue // missing directory is fine
205	205		}
206		-	for _, e := range entries {
207		-	if !e.IsDir() {
208		-	continue
209		-	}
210		-	if seenDirs[e.Name()] {
	206	+	for _, extDir := range extDirs {
	207	+	name := filepath.Base(extDir)
	208	+	if seenDirs[name] {
211	209		continue // higher-priority location already queued
212	210		}
213		-	seenDirs[e.Name()] = true
214		-	jobs = append(jobs, loadJob{dir: filepath.Join(dir, e.Name())})
	211	+	seenDirs[name] = true
	212	+	jobs = append(jobs, loadJob{dir: extDir})
215	213		}
216	214		}
217	215

@@ -250,6 +248,31 @@ func (m *Manager) searchDirs() []string {
250	248		return dirs
251	249		}
252	250
	251	+	// extensionDirs returns child directories, including directories reached
	252	+	// through symlinks. os.DirEntry.IsDir reports false for a directory symlink.
	253	+	func extensionDirs(root string) ([]string, error) {
	254	+	entries, err := os.ReadDir(root)
	255	+	if err != nil {
	256	+	return nil, err
	257	+	}
	258	+
	259	+	dirs := make([]string, 0, len(entries))
	260	+	for _, entry := range entries {
	261	+	path := filepath.Join(root, entry.Name())
	262	+	info, err := os.Stat(path)
	263	+	if err != nil {
	264	+	if os.IsNotExist(err) {
	265	+	continue
	266	+	}
	267	+	return nil, fmt.Errorf("stat %s: %w", path, err)
	268	+	}
	269	+	if info.IsDir() {
	270	+	dirs = append(dirs, path)
	271	+	}
	272	+	}
	273	+	return dirs, nil
	274	+	}
	275	+
253	276		// PlanSkillSources reads extension manifests without starting processes and
254	277		// returns their filesystem-backed skill roots in precedence order. Explicit
255	278		// extensions win over project and global installations; project wins global.

@@ -274,7 +297,7 @@ func PlanSkillSources(zotHome, cwd string, explicit []string, includeImplicit bo
274	297		var out []skills.Source
275	298		var errs []error
276	299		for _, root := range dirs {
277		-	entries, err := os.ReadDir(root)
	300	+	candidates, err := extensionDirs(root)
278	301		if err != nil {
279	302		if os.IsNotExist(err) {
280	303		continue

@@ -282,12 +305,6 @@ func PlanSkillSources(zotHome, cwd string, explicit []string, includeImplicit bo
282	305		errs = append(errs, fmt.Errorf("%s: %w", root, err))
283	306		continue
284	307		}
285		-	candidates := make([]string, 0, len(entries))
286		-	for _, e := range entries {
287		-	if e.IsDir() {
288		-	candidates = append(candidates, filepath.Join(root, e.Name()))
289		-	}
290		-	}
291	308		// An explicit --ext points at the extension itself, not its parent.
292	309		if filepath.Base(root) != "extensions" && filepath.Base(root) != ".zot" {
293	310		candidates = []string{root}

 	var jobs []loadJob
 	seenDirs := map[string]bool{} // dedup by basename so project wins
 	for _, dir := range m.searchDirs() {
-		entries, err := os.ReadDir(dir)
+		extDirs, err := extensionDirs(dir)
 		if err != nil {
 			continue // missing directory is fine
 		}
-		for _, e := range entries {
+		for _, extDir := range extDirs {
-			if !e.IsDir() {
+			name := filepath.Base(extDir)
-				continue
+			if seenDirs[name] {
-			}
-			if seenDirs[e.Name()] {
 				continue // higher-priority location already queued
 			}
-			seenDirs[e.Name()] = true
+			seenDirs[name] = true
-			jobs = append(jobs, loadJob{dir: filepath.Join(dir, e.Name())})
+			jobs = append(jobs, loadJob{dir: extDir})
 		}
 	}
 	return dirs
 }
+// extensionDirs returns child directories, including directories reached
+// through symlinks. os.DirEntry.IsDir reports false for a directory symlink.
+func extensionDirs(root string) ([]string, error) {
+	entries, err := os.ReadDir(root)
+	if err != nil {
+		return nil, err
+	}
+	dirs := make([]string, 0, len(entries))
+	for _, entry := range entries {
+		path := filepath.Join(root, entry.Name())
+		info, err := os.Stat(path)
+		if err != nil {
+			if os.IsNotExist(err) {
+				continue
+			}
+			return nil, fmt.Errorf("stat %s: %w", path, err)
+		}
+		if info.IsDir() {
+			dirs = append(dirs, path)
+		}
+	}
+	return dirs, nil
+}
 // PlanSkillSources reads extension manifests without starting processes and
 // returns their filesystem-backed skill roots in precedence order. Explicit
 // extensions win over project and global installations; project wins global.
 	var out []skills.Source
 	var errs []error
 	for _, root := range dirs {
-		entries, err := os.ReadDir(root)
+		candidates, err := extensionDirs(root)
 		if err != nil {
 			if os.IsNotExist(err) {
 				continue
 			errs = append(errs, fmt.Errorf("%s: %w", root, err))
 			continue
 		}
-		candidates := make([]string, 0, len(entries))
-		for _, e := range entries {
-			if e.IsDir() {
-				candidates = append(candidates, filepath.Join(root, e.Name()))
-			}
-		}
 		// An explicit --ext points at the extension itself, not its parent.
 		if filepath.Base(root) != "extensions" && filepath.Base(root) != ".zot" {
 			candidates = []string{root}

M packages/agent/extensions/manager_test.go +34 -0


@@ -219,6 +219,40 @@ func TestSpawnCleansUpProcessAfterInvalidHello(t *testing.T) {
219	219		}
220	220		}
221	221
	222	+	func TestPlanSkillSourcesFollowsSymlinkedExtension(t *testing.T) {
	223	+	if runtime.GOOS == "windows" {
	224	+	t.Skip("directory symlinks require elevated privileges on Windows")
	225	+	}
	226	+
	227	+	tmp := t.TempDir()
	228	+	extRoot := filepath.Join(tmp, "extensions")
	229	+	target := filepath.Join(tmp, "target")
	230	+	if err := os.MkdirAll(filepath.Join(target, "skills", "review"), 0o755); err != nil {
	231	+	t.Fatal(err)
	232	+	}
	233	+	if err := os.WriteFile(filepath.Join(target, "extension.json"), []byte(`{"name":"linked","skills":["./skills"]}`), 0o644); err != nil {
	234	+	t.Fatal(err)
	235	+	}
	236	+	if err := os.WriteFile(filepath.Join(target, "skills", "review", "SKILL.md"), []byte("---\ndescription: linked\n---\nbody"), 0o644); err != nil {
	237	+	t.Fatal(err)
	238	+	}
	239	+	if err := os.MkdirAll(extRoot, 0o755); err != nil {
	240	+	t.Fatal(err)
	241	+	}
	242	+	if err := os.Symlink(target, filepath.Join(extRoot, "linked")); err != nil {
	243	+	t.Fatal(err)
	244	+	}
	245	+
	246	+	sources, errs := PlanSkillSources(tmp, "", nil, true)
	247	+	if len(errs) != 0 \|\| len(sources) != 1 {
	248	+	t.Fatalf("sources=%#v errs=%v", sources, errs)
	249	+	}
	250	+	got, errs := skills.DiscoverSources(sources, false)
	251	+	if len(errs) != 0 \|\| len(got) != 1 \|\| got[0].Name != "linked:review" {
	252	+	t.Fatalf("skills=%#v errs=%v", got, errs)
	253	+	}
	254	+	}
	255	+
222	256		func TestPlanSkillSources(t *testing.T) {
223	257		tmp := t.TempDir()
224	258		extDir := filepath.Join(tmp, "bundle")


@@ -219,6 +219,40 @@ func TestSpawnCleansUpProcessAfterInvalidHello(t *testing.T) {
219	}	219		}
220	}	220		}
221		221
		222	+	func TestPlanSkillSourcesFollowsSymlinkedExtension(t *testing.T) {
		223	+	if runtime.GOOS == "windows" {
		224	+	t.Skip("directory symlinks require elevated privileges on Windows")
		225	+	}
		226	+
		227	+	tmp := t.TempDir()
		228	+	extRoot := filepath.Join(tmp, "extensions")
		229	+	target := filepath.Join(tmp, "target")
		230	+	if err := os.MkdirAll(filepath.Join(target, "skills", "review"), 0o755); err != nil {
		231	+	t.Fatal(err)
		232	+	}
		233	+	if err := os.WriteFile(filepath.Join(target, "extension.json"), []byte(`{"name":"linked","skills":["./skills"]}`), 0o644); err != nil {
		234	+	t.Fatal(err)
		235	+	}
		236	+	if err := os.WriteFile(filepath.Join(target, "skills", "review", "SKILL.md"), []byte("---\ndescription: linked\n---\nbody"), 0o644); err != nil {
		237	+	t.Fatal(err)
		238	+	}
		239	+	if err := os.MkdirAll(extRoot, 0o755); err != nil {
		240	+	t.Fatal(err)
		241	+	}
		242	+	if err := os.Symlink(target, filepath.Join(extRoot, "linked")); err != nil {
		243	+	t.Fatal(err)
		244	+	}
		245	+
		246	+	sources, errs := PlanSkillSources(tmp, "", nil, true)
		247	+	if len(errs) != 0 \|\| len(sources) != 1 {
		248	+	t.Fatalf("sources=%#v errs=%v", sources, errs)
		249	+	}
		250	+	got, errs := skills.DiscoverSources(sources, false)
		251	+	if len(errs) != 0 \|\| len(got) != 1 \|\| got[0].Name != "linked:review" {
		252	+	t.Fatalf("skills=%#v errs=%v", got, errs)
		253	+	}
		254	+	}
		255	+
222	func TestPlanSkillSources(t *testing.T) {	256		func TestPlanSkillSources(t *testing.T) {
223	tmp := t.TempDir()	257		tmp := t.TempDir()
224	extDir := filepath.Join(tmp, "bundle")	258		extDir := filepath.Join(tmp, "bundle")

M packages/agent/skills/skills.go +7 -3


@@ -187,7 +187,11 @@ func scanSource(source Source) ([]*Skill, []error) {
187	187		}
188	188		return []*Skill{s}, nil
189	189		}
190		-	err = filepath.WalkDir(source.Root, func(path string, d os.DirEntry, walkErr error) error {
	190	+	root, err := filepath.EvalSymlinks(source.Root)
	191	+	if err != nil {
	192	+	return out, []error{fmt.Errorf("resolve %s: %w", source.Root, err)}
	193	+	}
	194	+	err = filepath.WalkDir(root, func(path string, d os.DirEntry, walkErr error) error {
191	195		if walkErr != nil {
192	196		errs = append(errs, fmt.Errorf("%s: %w", path, walkErr))
193	197		return nil

@@ -201,7 +205,7 @@ func scanSource(source Source) ([]*Skill, []error) {
201	205		return nil
202	206		}
203	207		if s.Name == "" {
204		-	rel, e := filepath.Rel(source.Root, filepath.Dir(path))
	208	+	rel, e := filepath.Rel(root, filepath.Dir(path))
205	209		if e != nil {
206	210		errs = append(errs, e)
207	211		return nil

@@ -226,7 +230,7 @@ func scanSource(source Source) ([]*Skill, []error) {
226	230		return nil
227	231		})
228	232		if err != nil {
229		-	errs = append(errs, fmt.Errorf("scan %s: %w", source.Root, err))
	233	+	errs = append(errs, fmt.Errorf("scan %s: %w", root, err))
230	234		}
231	235		return out, errs
232	236		}

 		}
 		return []*Skill{s}, nil
 	}
-	err = filepath.WalkDir(source.Root, func(path string, d os.DirEntry, walkErr error) error {
+	root, err := filepath.EvalSymlinks(source.Root)
+	if err != nil {
+		return out, []error{fmt.Errorf("resolve %s: %w", source.Root, err)}
+	}
+	err = filepath.WalkDir(root, func(path string, d os.DirEntry, walkErr error) error {
 		if walkErr != nil {
 			errs = append(errs, fmt.Errorf("%s: %w", path, walkErr))
 			return nil
 			return nil
 		}
 		if s.Name == "" {
-			rel, e := filepath.Rel(source.Root, filepath.Dir(path))
+			rel, e := filepath.Rel(root, filepath.Dir(path))
 			if e != nil {
 				errs = append(errs, e)
 				return nil
 		return nil
 	})
 	if err != nil {
-		errs = append(errs, fmt.Errorf("scan %s: %w", source.Root, err))
+		errs = append(errs, fmt.Errorf("scan %s: %w", root, err))
 	}
 	return out, errs
 }

M packages/agent/skills/skills_test.go +25 -0


@@ -3,6 +3,7 @@ package skills
3	3		import (
4	4		"os"
5	5		"path/filepath"
	6	+	"runtime"
6	7		"strings"
7	8		"testing"
8	9		)

@@ -110,6 +111,30 @@ func TestKebabCase(t *testing.T) {
110	111		}
111	112		}
112	113
	114	+	func TestDiscoverSourcesFollowsSymlinkedRoot(t *testing.T) {
	115	+	if runtime.GOOS == "windows" {
	116	+	t.Skip("directory symlinks require elevated privileges on Windows")
	117	+	}
	118	+
	119	+	target := t.TempDir()
	120	+	path := filepath.Join(target, "nested", "review")
	121	+	if err := os.MkdirAll(path, 0o755); err != nil {
	122	+	t.Fatal(err)
	123	+	}
	124	+	if err := os.WriteFile(filepath.Join(path, "SKILL.md"), []byte("---\ndescription: linked\n---\nbody"), 0o644); err != nil {
	125	+	t.Fatal(err)
	126	+	}
	127	+	root := filepath.Join(t.TempDir(), "skills")
	128	+	if err := os.Symlink(target, root); err != nil {
	129	+	t.Fatal(err)
	130	+	}
	131	+
	132	+	got, errs := DiscoverSources([]Source{{Root: root, Prefix: "linked:"}}, false)
	133	+	if len(errs) != 0 \|\| len(got) != 1 \|\| got[0].Name != "linked:nested-review" {
	134	+	t.Fatalf("skills=%#v errs=%v", got, errs)
	135	+	}
	136	+	}
	137	+
113	138		func TestDiscoverSourcesRecursiveAndPrefixed(t *testing.T) {
114	139		root := t.TempDir()
115	140		path := filepath.Join(root, "Something", "Writing Git Commits")


@@ -3,6 +3,7 @@ package skills
3	import (	3		import (
4	"os"	4		"os"
5	"path/filepath"	5		"path/filepath"
		6	+	"runtime"
6	"strings"	7		"strings"
7	"testing"	8		"testing"
8	)	9		)

@@ -110,6 +111,30 @@ func TestKebabCase(t *testing.T) {
110	}	111		}
111	}	112		}
112		113
		114	+	func TestDiscoverSourcesFollowsSymlinkedRoot(t *testing.T) {
		115	+	if runtime.GOOS == "windows" {
		116	+	t.Skip("directory symlinks require elevated privileges on Windows")
		117	+	}
		118	+
		119	+	target := t.TempDir()
		120	+	path := filepath.Join(target, "nested", "review")
		121	+	if err := os.MkdirAll(path, 0o755); err != nil {
		122	+	t.Fatal(err)
		123	+	}
		124	+	if err := os.WriteFile(filepath.Join(path, "SKILL.md"), []byte("---\ndescription: linked\n---\nbody"), 0o644); err != nil {
		125	+	t.Fatal(err)
		126	+	}
		127	+	root := filepath.Join(t.TempDir(), "skills")
		128	+	if err := os.Symlink(target, root); err != nil {
		129	+	t.Fatal(err)
		130	+	}
		131	+
		132	+	got, errs := DiscoverSources([]Source{{Root: root, Prefix: "linked:"}}, false)
		133	+	if len(errs) != 0 \|\| len(got) != 1 \|\| got[0].Name != "linked:nested-review" {
		134	+	t.Fatalf("skills=%#v errs=%v", got, errs)
		135	+	}
		136	+	}
		137	+
113	func TestDiscoverSourcesRecursiveAndPrefixed(t *testing.T) {	138		func TestDiscoverSourcesRecursiveAndPrefixed(t *testing.T) {
114	root := t.TempDir()	139		root := t.TempDir()
115	path := filepath.Join(root, "Something", "Writing Git Commits")	140		path := filepath.Join(root, "Something", "Writing Git Commits")