feat: complete v0.3.1 agent workflow

2026-09-15 03:39:22 -07:00
parent 47671f3098
commit 065b30a84c
33 changed files with 4921 additions and 170 deletions
@@ -0,0 +1,583 @@
+package skills
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"net/http"
+	"os"
+	"path/filepath"
+	"regexp"
+	"sort"
+	"strings"
+	"time"
+
+	"agentu/internal/safeio"
+	"agentu/pkg/config"
+
+	"gopkg.in/yaml.v3"
+)
+
+// DefaultDir is the default directory for global skills.
+const DefaultDir = "~/.agentu/skills"
+
+// SkillFileName is the required instruction file inside a Codex-style skill directory.
+const SkillFileName = "SKILL.md"
+
+// maxInstallDownloadBytes caps HTTP downloads at 1 MiB.
+const maxInstallDownloadBytes = 1 << 20
+
+// Skill is a single global Markdown skill.
+type Skill struct {
+	Name        string
+	Description string
+	Path        string
+	Content     string
+}
+
+// InstallResult describes a successful skill installation.
+type InstallResult struct {
+	Skill     Skill
+	TargetDir string
+}
+
+// validNameRe allows ASCII letters, digits, dots, underscores, and hyphens.
+var validNameRe = regexp.MustCompile(`^[A-Za-z0-9._-]+$`)
+
+// validateSkillName rejects empty, path-like, and otherwise unsafe names.
+func validateSkillName(name string) error {
+	if name == "" || name == "." || name == ".." {
+		return fmt.Errorf("invalid skill name %q: use letters, numbers, dots, underscores, or hyphens", name)
+	}
+	if strings.ContainsAny(name, `/\`) {
+		return fmt.Errorf("invalid skill name %q: use letters, numbers, dots, underscores, or hyphens", name)
+	}
+	if strings.ContainsAny(name, " \t\n\r") {
+		return fmt.Errorf("invalid skill name %q: use letters, numbers, dots, underscores, or hyphens", name)
+	}
+	if !validNameRe.MatchString(name) {
+		return fmt.Errorf("invalid skill name %q: use letters, numbers, dots, underscores, or hyphens", name)
+	}
+	return nil
+}
+
+// skillFrontmatter is the decoded YAML frontmatter.
+type skillFrontmatter struct {
+	Name        string `yaml:"name"`
+	Description string `yaml:"description"`
+}
+
+// parseSkillFile reads raw bytes and returns the skill body, frontmatter name,
+// and frontmatter description. hasFrontmatter indicates whether the source
+// bytes started with a --- delimiter pair.
+func parseSkillFile(path string, data []byte) (body string, fmName string, fmDesc string, hasFrontmatter bool, err error) {
+	normalized := strings.ReplaceAll(string(data), "\r\n", "\n")
+	lines := strings.Split(normalized, "\n")
+	if len(lines) >= 2 && lines[0] == "---" {
+		for i := 1; i < len(lines); i++ {
+			if lines[i] == "---" {
+				yamlBlock := strings.Join(lines[1:i], "\n")
+				var fm skillFrontmatter
+				if err := yaml.Unmarshal([]byte(yamlBlock), &fm); err != nil {
+					return "", "", "", false, fmt.Errorf("parse skill %s: %w", path, err)
+				}
+				bodyText := strings.TrimSpace(strings.Join(lines[i+1:], "\n"))
+				return bodyText, fm.Name, fm.Description, true, nil
+			}
+		}
+		return "", "", "", false, fmt.Errorf("parse skill %s: missing closing ---", path)
+	}
+	return strings.TrimSpace(normalized), "", "", false, nil
+}
+
+// LoadGlobal loads global skills from DefaultDir.
+func LoadGlobal() ([]Skill, error) {
+	return LoadDir(DefaultDir)
+}
+
+// LoadDir loads skills from a directory. Missing directories return nil, nil.
+func LoadDir(dir string) ([]Skill, error) {
+	resolved, err := config.ResolvePath(dir)
+	if err != nil {
+		return nil, err
+	}
+	resolved, err = filepath.Abs(resolved)
+	if err != nil {
+		return nil, err
+	}
+
+	info, err := os.Stat(resolved)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, nil
+		}
+		return nil, fmt.Errorf("read skills directory: %w", err)
+	}
+	if !info.IsDir() {
+		return nil, fmt.Errorf("skills path is not a directory: %s", resolved)
+	}
+
+	entries, err := os.ReadDir(resolved)
+	if err != nil {
+		return nil, fmt.Errorf("read skills directory: %w", err)
+	}
+
+	var skills []Skill
+	for _, entry := range entries {
+		name := entry.Name()
+		if strings.HasPrefix(name, ".") {
+			continue
+		}
+
+		entryPath := filepath.Join(resolved, name)
+
+		// Shape 1: Codex-style directory with SKILL.md.
+		isDir, err := isDirectory(entryPath)
+		if err != nil {
+			return nil, err
+		}
+		if isDir {
+			skillFile := filepath.Join(entryPath, SkillFileName)
+			fi, err := os.Stat(skillFile)
+			if err != nil {
+				if os.IsNotExist(err) {
+					continue
+				}
+				return nil, fmt.Errorf("read skill %s: %w", skillFile, err)
+			}
+			if !fi.Mode().IsRegular() {
+				continue
+			}
+			skill, err := loadSkillFile(name, skillFile, true)
+			if err != nil {
+				return nil, err
+			}
+			if skill != nil {
+				skills = append(skills, *skill)
+			}
+			continue
+		}
+
+		// Shape 2: Legacy flat .md file.
+		if !entry.Type().IsRegular() {
+			continue
+		}
+		if !strings.EqualFold(filepath.Ext(name), ".md") {
+			continue
+		}
+		skill, err := loadSkillFile(name[:len(name)-len(filepath.Ext(name))], entryPath, false)
+		if err != nil {
+			return nil, err
+		}
+		if skill != nil {
+			skills = append(skills, *skill)
+		}
+	}
+
+	sort.Slice(skills, func(i, j int) bool {
+		if skills[i].Name != skills[j].Name {
+			return skills[i].Name < skills[j].Name
+		}
+		return skills[i].Path < skills[j].Path
+	})
+
+	return skills, nil
+}
+
+// isDirectory reports whether path is a directory. For symlinks it resolves
+// the target and checks the resolved type.
+func isDirectory(path string) (bool, error) {
+	fi, err := os.Lstat(path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return false, nil
+		}
+		return false, fmt.Errorf("read skill %s: %w", path, err)
+	}
+	if fi.IsDir() {
+		return true, nil
+	}
+	if fi.Mode()&os.ModeSymlink != 0 {
+		resolved, err := filepath.EvalSymlinks(path)
+		if err != nil {
+			return false, nil
+		}
+		target, err := os.Stat(resolved)
+		if err != nil {
+			return false, nil
+		}
+		return target.IsDir(), nil
+	}
+	return false, nil
+}
+
+// loadSkillFile reads one skill markdown file and returns a Skill or nil when
+// the instruction body is empty. legacyBaseName is used as the default name
+// when frontmatter does not supply one and isDir is false.
+func loadSkillFile(legacyBaseName string, path string, isDirSkill bool) (*Skill, error) {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return nil, fmt.Errorf("read skill %s: %w", path, err)
+	}
+	body, fmName, fmDesc, _, err := parseSkillFile(path, data)
+	if err != nil {
+		return nil, err
+	}
+	if body == "" {
+		return nil, nil
+	}
+
+	name := fmName
+	if name == "" {
+		if isDirSkill {
+			name = filepath.Base(filepath.Dir(path))
+		} else {
+			name = legacyBaseName
+		}
+	}
+	if err := validateSkillName(name); err != nil {
+		return nil, err
+	}
+
+	return &Skill{
+		Name:        name,
+		Description: strings.TrimSpace(fmDesc),
+		Path:        path,
+		Content:     body,
+	}, nil
+}
+
+// FormatForSystemPrompt formats skills for injection into the system prompt.
+// Returns an empty string if items is empty.
+func FormatForSystemPrompt(items []Skill) string {
+	if len(items) == 0 {
+		return ""
+	}
+	var b strings.Builder
+	b.WriteString("Global skills:\n")
+	b.WriteString("The following global skills are available for every conversation. Apply them when relevant.")
+	for _, skill := range items {
+		b.WriteString("\n\n### ")
+		b.WriteString(skill.Name)
+		b.WriteString("\nSource: ")
+		b.WriteString(skill.Path)
+		if skill.Description != "" {
+			b.WriteString("\nDescription: ")
+			b.WriteString(skill.Description)
+		}
+		b.WriteString("\n\n")
+		b.WriteString(skill.Content)
+	}
+	return b.String()
+}
+
+// FormatList formats skills for slash-command output.
+func FormatList(items []Skill) string {
+	if len(items) == 0 {
+		return "No global skills installed."
+	}
+	var b strings.Builder
+	b.WriteString("Global skills:\n")
+	for _, skill := range items {
+		b.WriteString("- ")
+		b.WriteString(skill.Name)
+		if skill.Description != "" {
+			b.WriteString(" — ")
+			b.WriteString(skill.Description)
+		}
+		b.WriteString("\n  Source: ")
+		b.WriteString(skill.Path)
+		b.WriteString("\n")
+	}
+	return b.String()
+}
+
+// InstallGlobal installs a skill into DefaultDir.
+func InstallGlobal(ctx context.Context, source string) (InstallResult, error) {
+	return InstallDir(ctx, DefaultDir, source)
+}
+
+// InstallDir installs a Codex-style skill directory under the target root.
+func InstallDir(ctx context.Context, dir string, source string) (InstallResult, error) {
+	source = strings.TrimSpace(source)
+	if source == "" {
+		return InstallResult{}, fmt.Errorf("skill source is required")
+	}
+
+	resolvedRoot, err := config.ResolvePath(dir)
+	if err != nil {
+		return InstallResult{}, err
+	}
+	resolvedRoot, err = filepath.Abs(resolvedRoot)
+	if err != nil {
+		return InstallResult{}, err
+	}
+
+	// HTTP(S) source.
+	if strings.HasPrefix(source, "http://") || strings.HasPrefix(source, "https://") {
+		return installFromHTTP(ctx, resolvedRoot, source)
+	}
+
+	// Local source.
+	resolvedSource, err := config.ResolvePath(source)
+	if err != nil {
+		return InstallResult{}, err
+	}
+	resolvedSource, err = filepath.Abs(resolvedSource)
+	if err != nil {
+		return InstallResult{}, err
+	}
+
+	fi, err := os.Stat(resolvedSource)
+	if err != nil {
+		return InstallResult{}, fmt.Errorf("read skill source: %w", err)
+	}
+	if fi.IsDir() {
+		return installFromDirectory(ctx, resolvedRoot, resolvedSource)
+	}
+	if fi.Mode().IsRegular() && strings.EqualFold(filepath.Ext(resolvedSource), ".md") {
+		return installFromFile(ctx, resolvedRoot, resolvedSource)
+	}
+	return InstallResult{}, fmt.Errorf("skill source must be a directory, .md file, or http(s) URL")
+}
+
+func installFromHTTP(ctx context.Context, root string, url string) (InstallResult, error) {
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+	if err != nil {
+		return InstallResult{}, fmt.Errorf("download skill: %w", err)
+	}
+	client := &http.Client{Timeout: 30 * time.Second}
+	resp, err := client.Do(req)
+	if err != nil {
+		return InstallResult{}, fmt.Errorf("download skill: %w", err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		return InstallResult{}, fmt.Errorf("download skill: HTTP %d", resp.StatusCode)
+	}
+	limited := io.LimitReader(resp.Body, int64(maxInstallDownloadBytes)+1)
+	data, err := io.ReadAll(limited)
+	if err != nil {
+		return InstallResult{}, fmt.Errorf("download skill: %w", err)
+	}
+	if len(data) > maxInstallDownloadBytes {
+		return InstallResult{}, fmt.Errorf("skill file is too large: max %d bytes", maxInstallDownloadBytes)
+	}
+	return installMarkdownBytes(root, data, deriveNameFromURL(url))
+}
+
+func installFromDirectory(ctx context.Context, root string, sourceDir string) (InstallResult, error) {
+	skillFile := filepath.Join(sourceDir, SkillFileName)
+	fi, err := os.Stat(skillFile)
+	if err != nil {
+		return InstallResult{}, fmt.Errorf("read skill source: %w", err)
+	}
+	if !fi.Mode().IsRegular() {
+		return InstallResult{}, fmt.Errorf("skill source must contain a regular %s file", SkillFileName)
+	}
+	data, err := os.ReadFile(skillFile)
+	if err != nil {
+		return InstallResult{}, fmt.Errorf("read skill source: %w", err)
+	}
+
+	body, fmName, _, _, err := parseSkillFile(skillFile, data)
+	if err != nil {
+		return InstallResult{}, err
+	}
+	_ = body
+	name := fmName
+	if name == "" {
+		name = filepath.Base(sourceDir)
+	}
+	if err := validateSkillName(name); err != nil {
+		return InstallResult{}, err
+	}
+
+	targetDir := filepath.Join(root, name)
+	if err := createSkillTarget(root, targetDir); err != nil {
+		return InstallResult{}, err
+	}
+
+	// Copy tree.
+	if err := copyTree(ctx, sourceDir, targetDir); err != nil {
+		os.RemoveAll(targetDir)
+		return InstallResult{}, err
+	}
+
+	installedPath := filepath.Join(targetDir, SkillFileName)
+	installedData, _ := os.ReadFile(installedPath)
+	instBody, instName, instDesc, _, _ := parseSkillFile(installedPath, installedData)
+	if instName == "" {
+		instName = name
+	}
+
+	return InstallResult{
+		Skill: Skill{
+			Name:        instName,
+			Description: strings.TrimSpace(instDesc),
+			Path:        installedPath,
+			Content:     instBody,
+		},
+		TargetDir: targetDir,
+	}, nil
+}
+
+func installFromFile(ctx context.Context, root string, sourcePath string) (InstallResult, error) {
+	data, err := os.ReadFile(sourcePath)
+	if err != nil {
+		return InstallResult{}, fmt.Errorf("read skill source: %w", err)
+	}
+	baseName := filepath.Base(sourcePath)
+	baseNameNoExt := baseName[:len(baseName)-len(filepath.Ext(baseName))]
+	// When the source is literally SKILL.md, use the parent directory name.
+	if strings.EqualFold(baseName, SkillFileName) {
+		baseNameNoExt = filepath.Base(filepath.Dir(sourcePath))
+	}
+	return installMarkdownBytes(root, data, baseNameNoExt)
+}
+
+// installMarkdownBytes parses markdown bytes and writes a Codex-style
+// SKILL.md under <root>/<name>/. If the source had frontmatter, the original
+// bytes are preserved. Otherwise a normalized header is prepended.
+func installMarkdownBytes(root string, data []byte, fallbackName string) (InstallResult, error) {
+	normalized := strings.ReplaceAll(string(data), "\r\n", "\n")
+	lines := strings.Split(normalized, "\n")
+
+	hasFrontmatter := false
+	if len(lines) >= 2 && lines[0] == "---" {
+		for i := 1; i < len(lines); i++ {
+			if lines[i] == "---" {
+				hasFrontmatter = true
+				break
+			}
+		}
+	}
+
+	var body string
+	var fmName string
+	var fmDesc string
+	var err error
+
+	if hasFrontmatter {
+		body, fmName, fmDesc, _, err = parseSkillFile("<source>", data)
+		if err != nil {
+			return InstallResult{}, err
+		}
+	} else {
+		body = strings.TrimSpace(normalized)
+	}
+	name := fmName
+	if name == "" {
+		name = fallbackName
+	}
+	if err := validateSkillName(name); err != nil {
+		return InstallResult{}, err
+	}
+	if body == "" {
+		return InstallResult{}, fmt.Errorf("skill body is empty")
+	}
+
+	targetDir := filepath.Join(root, name)
+	if err := createSkillTarget(root, targetDir); err != nil {
+		return InstallResult{}, err
+	}
+
+	var writeData []byte
+	if hasFrontmatter {
+		writeData = data
+	} else {
+		var b strings.Builder
+		b.WriteString("---\n")
+		fmt.Fprintf(&b, "name: %s\n", name)
+		b.WriteString("description: \"\"\n")
+		b.WriteString("---\n\n")
+		b.WriteString(body)
+		b.WriteString("\n")
+		writeData = []byte(b.String())
+	}
+
+	targetFile := filepath.Join(targetDir, SkillFileName)
+	if err := safeio.AtomicWriteFile(targetFile, writeData, 0o644); err != nil {
+		os.RemoveAll(targetDir)
+		return InstallResult{}, err
+	}
+
+	return InstallResult{
+		Skill: Skill{
+			Name:        name,
+			Description: strings.TrimSpace(fmDesc),
+			Path:        targetFile,
+			Content:     body,
+		},
+		TargetDir: targetDir,
+	}, nil
+}
+
+func createSkillTarget(root, targetDir string) error {
+	if err := os.MkdirAll(root, 0o755); err != nil {
+		return fmt.Errorf("create skills directory: %w", err)
+	}
+	if err := os.Mkdir(targetDir, 0o755); err != nil {
+		if os.IsExist(err) {
+			return fmt.Errorf("skill already exists: %s", targetDir)
+		}
+		return fmt.Errorf("create skill directory: %w", err)
+	}
+	return nil
+}
+
+// deriveNameFromURL extracts a filename stem from a URL for use as a fallback name.
+func deriveNameFromURL(rawURL string) string {
+	// Strip query and fragment.
+	u := rawURL
+	if idx := strings.IndexAny(u, "?#"); idx >= 0 {
+		u = u[:idx]
+	}
+	base := filepath.Base(u)
+	ext := filepath.Ext(base)
+	if ext != "" {
+		base = base[:len(base)-len(ext)]
+	}
+	if base == "" || strings.EqualFold(base, "SKILL") {
+		return "downloaded-skill"
+	}
+	return base
+}
+
+// copyTree copies srcDir to dstDir, rejecting symlinks inside the source tree.
+func copyTree(ctx context.Context, srcDir string, dstDir string) error {
+	return filepath.Walk(srcDir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if err := ctx.Err(); err != nil {
+			return err
+		}
+		rel, err := filepath.Rel(srcDir, path)
+		if err != nil {
+			return err
+		}
+		target := filepath.Join(dstDir, rel)
+
+		// Reject any symlink inside the source tree.
+		lfi, lerr := os.Lstat(path)
+		if lerr != nil {
+			return lerr
+		}
+		if lfi.Mode()&os.ModeSymlink != 0 {
+			return fmt.Errorf("skill source contains symlink: %s", path)
+		}
+
+		if lfi.IsDir() {
+			return os.MkdirAll(target, 0o755)
+		}
+		if !lfi.Mode().IsRegular() {
+			return nil
+		}
+		data, err := os.ReadFile(path)
+		if err != nil {
+			return err
+		}
+		return safeio.AtomicWriteFile(target, data, 0o644)
+	})
+}
@@ -0,0 +1,404 @@
+package skills
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestLoadDirMissingReturnsEmpty(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), "missing")
+	skills, err := LoadDir(dir)
+	if err != nil {
+		t.Fatalf("LoadDir(missing) returned error: %v", err)
+	}
+	if len(skills) != 0 {
+		t.Fatalf("expected 0 skills, got %d", len(skills))
+	}
+}
+
+func TestLoadDirRejectsFilePath(t *testing.T) {
+	file := filepath.Join(t.TempDir(), "not-a-dir")
+	if err := os.WriteFile(file, []byte("x"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	_, err := LoadDir(file)
+	if err == nil {
+		t.Fatal("expected error for file path")
+	}
+	if !strings.Contains(err.Error(), "skills path is not a directory") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+func TestLoadDirLoadsDirectorySkillsSorted(t *testing.T) {
+	dir := t.TempDir()
+
+	// b-skill loads second by name.
+	bDir := filepath.Join(dir, "b-skill")
+	if err := os.MkdirAll(bDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(bDir, SkillFileName), []byte("---\nname: b-skill\ndescription: Beta skill\n---\n\nBeta content.\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	// a-skill loads first by name.
+	aDir := filepath.Join(dir, "a-skill")
+	if err := os.MkdirAll(aDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(aDir, SkillFileName), []byte("---\nname: a-skill\ndescription: Alpha skill\n---\n\nAlpha content.\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	// Visible directory without SKILL.md — ignored.
+	noSkillDir := filepath.Join(dir, "no-skill-here")
+	if err := os.MkdirAll(noSkillDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(noSkillDir, "README.md"), []byte("not a skill"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	skills, err := LoadDir(dir)
+	if err != nil {
+		t.Fatalf("LoadDir: %v", err)
+	}
+	if len(skills) != 2 {
+		t.Fatalf("expected 2 skills, got %d: %+v", len(skills), skills)
+	}
+
+	if skills[0].Name != "a-skill" {
+		t.Fatalf("skills[0].Name = %q", skills[0].Name)
+	}
+	if skills[0].Description != "Alpha skill" {
+		t.Fatalf("skills[0].Description = %q", skills[0].Description)
+	}
+	if skills[0].Content != "Alpha content." {
+		t.Fatalf("skills[0].Content = %q", skills[0].Content)
+	}
+	if !strings.HasSuffix(skills[0].Path, SkillFileName) {
+		t.Fatalf("skills[0].Path = %q, want suffix %s", skills[0].Path, SkillFileName)
+	}
+
+	if skills[1].Name != "b-skill" {
+		t.Fatalf("skills[1].Name = %q", skills[1].Name)
+	}
+	if skills[1].Description != "Beta skill" {
+		t.Fatalf("skills[1].Description = %q", skills[1].Description)
+	}
+}
+
+func TestLoadDirKeepsLegacyMarkdownSkills(t *testing.T) {
+	dir := t.TempDir()
+
+	write := func(name, content string) {
+		if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	write("legacy.md", "  legacy content  ")
+	write(".hidden.md", "hidden")
+	write("notes.txt", "not a skill")
+	write("empty.md", "  \n  ")
+
+	skills, err := LoadDir(dir)
+	if err != nil {
+		t.Fatalf("LoadDir: %v", err)
+	}
+	if len(skills) != 1 {
+		t.Fatalf("expected 1 skill, got %d: %+v", len(skills), skills)
+	}
+	if skills[0].Name != "legacy" {
+		t.Fatalf("skill.Name = %q", skills[0].Name)
+	}
+	if skills[0].Content != "legacy content" {
+		t.Fatalf("skill.Content = %q", skills[0].Content)
+	}
+}
+
+func TestInstallDirInstallsMarkdownFileAsSkillDirectory(t *testing.T) {
+	sourceDir := t.TempDir()
+	targetRoot := filepath.Join(t.TempDir(), "skills")
+
+	sourcePath := filepath.Join(sourceDir, "source.md")
+	sourceContent := "---\nname: uiux-pro-max\ndescription: UI workflow\n---\n\nReview the UI carefully.\n"
+	if err := os.WriteFile(sourcePath, []byte(sourceContent), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	result, err := InstallDir(context.Background(), targetRoot, sourcePath)
+	if err != nil {
+		t.Fatalf("InstallDir: %v", err)
+	}
+
+	absRoot, _ := filepath.Abs(targetRoot)
+	wantDir := filepath.Join(absRoot, "uiux-pro-max")
+	if result.TargetDir != wantDir {
+		t.Fatalf("TargetDir = %q, want %q", result.TargetDir, wantDir)
+	}
+
+	skillFile := filepath.Join(wantDir, SkillFileName)
+	if _, err := os.Stat(skillFile); err != nil {
+		t.Fatalf("SKILL.md not created: %v", err)
+	}
+
+	loaded, err := LoadDir(targetRoot)
+	if err != nil {
+		t.Fatalf("LoadDir: %v", err)
+	}
+	if len(loaded) != 1 {
+		t.Fatalf("expected 1 loaded skill, got %d", len(loaded))
+	}
+	if loaded[0].Name != "uiux-pro-max" {
+		t.Fatalf("loaded name = %q", loaded[0].Name)
+	}
+	if loaded[0].Content != "Review the UI carefully." {
+		t.Fatalf("loaded content = %q", loaded[0].Content)
+	}
+}
+
+func TestInstallDirCopiesDirectorySkill(t *testing.T) {
+	sourceDir := t.TempDir()
+	targetRoot := filepath.Join(t.TempDir(), "skills")
+
+	skillSrc := filepath.Join(sourceDir, "grill-me")
+	if err := os.MkdirAll(filepath.Join(skillSrc, "references"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(skillSrc, SkillFileName), []byte("---\nname: grill-me\ndescription: Grill skill\n---\n\nGrill content.\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(skillSrc, "references", "guide.md"), []byte("# Guide\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	result, err := InstallDir(context.Background(), targetRoot, skillSrc)
+	if err != nil {
+		t.Fatalf("InstallDir: %v", err)
+	}
+
+	skillFile := filepath.Join(result.TargetDir, SkillFileName)
+	if _, err := os.Stat(skillFile); err != nil {
+		t.Fatalf("SKILL.md not copied: %v", err)
+	}
+	guideFile := filepath.Join(result.TargetDir, "references", "guide.md")
+	if _, err := os.Stat(guideFile); err != nil {
+		t.Fatalf("references/guide.md not copied: %v", err)
+	}
+
+	loaded, err := LoadDir(targetRoot)
+	if err != nil {
+		t.Fatalf("LoadDir: %v", err)
+	}
+	if len(loaded) != 1 || loaded[0].Name != "grill-me" {
+		t.Fatalf("loaded = %+v", loaded)
+	}
+}
+
+func TestInstallDirRejectsExistingSkill(t *testing.T) {
+	targetRoot := filepath.Join(t.TempDir(), "skills")
+	existingDir := filepath.Join(targetRoot, "grill-me")
+	if err := os.MkdirAll(existingDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	sourceDir := t.TempDir()
+	sourcePath := filepath.Join(sourceDir, "source.md")
+	if err := os.WriteFile(sourcePath, []byte("---\nname: grill-me\ndescription: d\n---\n\nbody\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	_, err := InstallDir(context.Background(), targetRoot, sourcePath)
+	if err == nil {
+		t.Fatal("expected error for existing skill")
+	}
+	if !strings.Contains(err.Error(), "skill already exists") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+func TestInstallDirRejectsInvalidName(t *testing.T) {
+	targetRoot := filepath.Join(t.TempDir(), "skills")
+	sourceDir := t.TempDir()
+	sourcePath := filepath.Join(sourceDir, "source.md")
+	if err := os.WriteFile(sourcePath, []byte("---\nname: ../bad\ndescription: d\n---\n\nbody\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	_, err := InstallDir(context.Background(), targetRoot, sourcePath)
+	if err == nil {
+		t.Fatal("expected error for invalid name")
+	}
+	if !strings.Contains(err.Error(), "invalid skill name") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+func TestInstallDirRequiresSource(t *testing.T) {
+	targetRoot := filepath.Join(t.TempDir(), "skills")
+	_, err := InstallDir(context.Background(), targetRoot, "")
+	if err == nil {
+		t.Fatal("expected error for empty source")
+	}
+	if !strings.Contains(err.Error(), "skill source is required") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+func TestInstallDirRejectsBadSourceType(t *testing.T) {
+	targetRoot := filepath.Join(t.TempDir(), "skills")
+	sourceDir := t.TempDir()
+	sourcePath := filepath.Join(sourceDir, "something.yaml")
+	if err := os.WriteFile(sourcePath, []byte("key: value\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	_, err := InstallDir(context.Background(), targetRoot, sourcePath)
+	if err == nil {
+		t.Fatal("expected error for non-.md non-dir source")
+	}
+	if !strings.Contains(err.Error(), "skill source must be a directory") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+func TestInstallDirWithoutFrontmatterCreatesNormalizedHeader(t *testing.T) {
+	targetRoot := filepath.Join(t.TempDir(), "skills")
+	sourceDir := t.TempDir()
+	sourcePath := filepath.Join(sourceDir, "my-skill.md")
+	if err := os.WriteFile(sourcePath, []byte("Just body content.\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	result, err := InstallDir(context.Background(), targetRoot, sourcePath)
+	if err != nil {
+		t.Fatalf("InstallDir: %v", err)
+	}
+	if result.Skill.Name != "my-skill" {
+		t.Fatalf("name = %q", result.Skill.Name)
+	}
+
+	installed, err := os.ReadFile(filepath.Join(result.TargetDir, SkillFileName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	content := string(installed)
+	if !strings.HasPrefix(content, "---\nname: my-skill\n") {
+		t.Fatalf("installed SKILL.md missing normalized header: %q", content)
+	}
+	if !strings.Contains(content, "Just body content.") {
+		t.Fatalf("installed SKILL.md missing body: %q", content)
+	}
+}
+
+func TestFormatForSystemPromptIncludesDescription(t *testing.T) {
+	items := []Skill{
+		{Name: "first", Path: "/tmp/first.md", Content: "skill one"},
+		{Name: "second", Description: "Described", Path: "/tmp/second.md", Content: "skill two"},
+	}
+	out := FormatForSystemPrompt(items)
+	if out == "" {
+		t.Fatal("FormatForSystemPrompt returned empty string")
+	}
+	if !strings.Contains(out, "Global skills:") {
+		t.Fatal("missing 'Global skills:' header")
+	}
+	if !strings.Contains(out, "### first") {
+		t.Fatal("missing '### first'")
+	}
+	if !strings.Contains(out, "### second") {
+		t.Fatal("missing '### second'")
+	}
+	if !strings.Contains(out, "Source: /tmp/first.md") {
+		t.Fatal("missing source for first")
+	}
+	if !strings.Contains(out, "Source: /tmp/second.md") {
+		t.Fatal("missing source for second")
+	}
+	if !strings.Contains(out, "skill one") {
+		t.Fatal("missing content for first")
+	}
+	if !strings.Contains(out, "skill two") {
+		t.Fatal("missing content for second")
+	}
+	// First skill has no description — the Description: line must be absent.
+	firstBlock := out[:strings.Index(out, "### second")]
+	if strings.Contains(firstBlock, "Description:") {
+		t.Fatal("first skill should not have Description line")
+	}
+	// Second skill has description — the line must appear.
+	if !strings.Contains(out, "Description: Described") {
+		t.Fatal("missing Description line for second skill")
+	}
+}
+
+func TestFormatForSystemPromptEmptyReturnsEmpty(t *testing.T) {
+	if got := FormatForSystemPrompt(nil); got != "" {
+		t.Fatalf("expected empty string, got %q", got)
+	}
+}
+
+func TestFormatListNonEmpty(t *testing.T) {
+	items := []Skill{
+		{Name: "alpha", Description: "Alpha desc", Path: "/tmp/alpha/SKILL.md", Content: "..."},
+		{Name: "beta", Path: "/tmp/beta/SKILL.md", Content: "..."},
+	}
+	out := FormatList(items)
+	if !strings.Contains(out, "Global skills:") {
+		t.Fatal("missing header")
+	}
+	if !strings.Contains(out, "- alpha — Alpha desc") {
+		t.Fatal("missing alpha with description")
+	}
+	if !strings.Contains(out, "  Source: /tmp/alpha/SKILL.md") {
+		t.Fatal("missing alpha source")
+	}
+	if !strings.Contains(out, "- beta\n") {
+		t.Fatal("missing beta without description")
+	}
+	// Ensure beta has no em-dash line.
+	if strings.Contains(out, "- beta —") {
+		t.Fatal("beta should not have description separator")
+	}
+}
+
+func TestFormatListEmpty(t *testing.T) {
+	out := FormatList(nil)
+	if out != "No global skills installed." {
+		t.Fatalf("unexpected: %q", out)
+	}
+}
+
+func TestLoadDirRejectsInvalidFrontmatterName(t *testing.T) {
+	dir := t.TempDir()
+	skillDir := filepath.Join(dir, "bad-skill")
+	if err := os.MkdirAll(skillDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(skillDir, SkillFileName), []byte("---\nname: ../bad\n---\n\nbody\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	_, err := LoadDir(dir)
+	if err == nil {
+		t.Fatal("expected error for invalid frontmatter name")
+	}
+	if !strings.Contains(err.Error(), "invalid skill name") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+func TestParseSkillFileMissingClosingDelimiter(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "bad.md")
+	_, _, _, _, err := parseSkillFile(path, []byte("---\nname: foo\nbody\n"))
+	if err == nil {
+		t.Fatal("expected error for missing closing ---")
+	}
+	if !strings.Contains(err.Error(), "missing closing ---") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}