Install portable agent-neutral Hop skill and documentation

Hop-State: A_06FN69X2VW0BAXG4A4DT6V0
Hop-Proposal: R_06FN69VET1STZNKPWK913K8
Hop-Task: T_06FN637799RW5Q7WH9H6PJR
Hop-Attempt: AT_06FN63779BJRBK1VFQ94GRR

internal/hop/cli.go

@@ -528,14 +528,22 @@ func runSkillCLI(args []string, jsonOutput bool, stdout, stderr io.Writer) int {
 		if err := fs.Parse(args[1:]); err != nil || len(fs.Args()) != 0 {
 			return 2
 		}
-		result, err := InstallSkill(*path, *force)
+		var result SkillInstallResult
+		var err error
+		if strings.TrimSpace(*path) == "" {
+			result, err = InstallDefaultSkills(*force)
+		} else {
+			result, err = InstallSkill(*path, *force)
+		}
 		if err != nil {
 			return printCLIError(err, jsonOutput, stdout, stderr)
 		}
 		if jsonOutput {
 			writeJSON(stdout, map[string]any{"ok": true, "data": result})
 		} else {
-			fmt.Fprintf(stdout, "Installed Hop skill at %s\n", result.Path)
+			for _, installedPath := range result.Paths {
+				fmt.Fprintf(stdout, "Installed Hop skill at %s\n", installedPath)
+			}
 		}
 		return 0
 	case "print":

internal/hop/skill.go

@@ -12,6 +12,7 @@ import (
 
 type SkillInstallResult struct {
 	Path  string   `json:"path"`
+	Paths []string `json:"paths,omitempty"`
 	Files []string `json:"files"`
 }
 
@@ -27,6 +28,26 @@ func DefaultSkillBase() (string, error) {
 	return filepath.Join(home, "skills"), nil
 }
 
+func DefaultSharedSkillBase() (string, error) {
+	home, err := os.UserHomeDir()
+	if err != nil {
+		return "", fmt.Errorf("find home directory: %w", err)
+	}
+	return filepath.Join(home, ".agents", "skills"), nil
+}
+
+func DefaultSkillBases() ([]string, error) {
+	codex, err := DefaultSkillBase()
+	if err != nil {
+		return nil, err
+	}
+	shared, err := DefaultSharedSkillBase()
+	if err != nil {
+		return nil, err
+	}
+	return []string{codex, shared}, nil
+}
+
 func EmbeddedSkillText() (string, error) {
 	contents, err := hopskill.Files.ReadFile("SKILL.md")
 	if err != nil {
@@ -35,9 +56,10 @@ func EmbeddedSkillText() (string, error) {
 	return string(contents), nil
 }
 
-// InstallSkill writes the embedded skill below a skills directory. Existing
+// InstallSkill writes the embedded skill below one skills directory. Existing
 // bundles require force; known files are overwritten without deleting unknown
-// user files from the destination.
+// user files from the destination. An empty base retains the legacy Codex
+// destination; the CLI uses InstallDefaultSkills for its no-path behavior.
 func InstallSkill(base string, force bool) (SkillInstallResult, error) {
 	if strings.TrimSpace(base) == "" {
 		var err error
@@ -46,30 +68,182 @@ func InstallSkill(base string, force bool) (SkillInstallResult, error) {
 			return SkillInstallResult{}, err
 		}
 	}
+	target, err := resolveSkillTarget(base)
+	if err != nil {
+		return SkillInstallResult{}, err
+	}
+	if err := preflightSkillTarget(target, force); err != nil {
+		return SkillInstallResult{}, err
+	}
+	return writeSkillTarget(target)
+}
+
+// InstallDefaultSkills installs the same embedded files in the client-specific
+// Codex directory and the cross-client .agents directory. Destinations are
+// canonicalized and preflighted before any write to avoid partial upgrades when
+// one existing bundle requires --force.
+func InstallDefaultSkills(force bool) (SkillInstallResult, error) {
+	bases, err := DefaultSkillBases()
+	if err != nil {
+		return SkillInstallResult{}, err
+	}
+	var targets []string
+	seen := make(map[string]struct{}, len(bases))
+	for _, base := range bases {
+		target, err := resolveSkillTarget(base)
+		if err != nil {
+			return SkillInstallResult{}, err
+		}
+		key, err := canonicalPathKey(target)
+		if err != nil {
+			return SkillInstallResult{}, err
+		}
+		if _, exists := seen[key]; exists {
+			continue
+		}
+		seen[key] = struct{}{}
+		targets = append(targets, target)
+	}
+	for _, target := range targets {
+		if err := preflightSkillTarget(target, force); err != nil {
+			return SkillInstallResult{}, err
+		}
+	}
+	var combined SkillInstallResult
+	for _, target := range targets {
+		installed, err := writeSkillTarget(target)
+		if err != nil {
+			return SkillInstallResult{}, err
+		}
+		if combined.Path == "" {
+			combined.Path = installed.Path
+			combined.Files = installed.Files
+		}
+		combined.Paths = append(combined.Paths, installed.Path)
+	}
+	return combined, nil
+}
+
+func resolveSkillTarget(base string) (string, error) {
 	absBase, err := filepath.Abs(base)
 	if err != nil {
-		return SkillInstallResult{}, fmt.Errorf("resolve skills directory: %w", err)
+		return "", fmt.Errorf("resolve skills directory: %w", err)
+	}
+	return filepath.Join(absBase, "hop"), nil
+}
+
+func canonicalPathKey(path string) (string, error) {
+	if err := preflightSkillAncestors(path); err != nil {
+		return "", err
+	}
+	current := filepath.Clean(path)
+	var suffix []string
+	for {
+		resolved, err := filepath.EvalSymlinks(current)
+		if err == nil {
+			for index := len(suffix) - 1; index >= 0; index-- {
+				resolved = filepath.Join(resolved, suffix[index])
+			}
+			return filepath.Clean(resolved), nil
+		}
+		if !os.IsNotExist(err) {
+			return "", fmt.Errorf("canonicalize skill destination: %w", err)
+		}
+		parent := filepath.Dir(current)
+		if parent == current {
+			return filepath.Clean(path), nil
+		}
+		suffix = append(suffix, filepath.Base(current))
+		current = parent
+	}
+}
+
+func preflightSkillTarget(target string, force bool) error {
+	if err := preflightSkillAncestors(target); err != nil {
+		return err
 	}
-	target := filepath.Join(absBase, "hop")
 	if info, err := os.Lstat(target); err == nil {
 		if info.Mode()&os.ModeSymlink != 0 {
-			return SkillInstallResult{}, fmt.Errorf("refusing to install through symlink %s", target)
+			return fmt.Errorf("refusing to install through symlink %s", target)
 		}
 		if !info.IsDir() {
-			return SkillInstallResult{}, fmt.Errorf("skill target exists and is not a directory: %s", target)
+			return fmt.Errorf("skill target exists and is not a directory: %s", target)
 		}
 		if !force {
-			return SkillInstallResult{}, fmt.Errorf("Hop skill already exists at %s; pass --force to update it", target)
+			return fmt.Errorf("Hop skill already exists at %s; pass --force to update it", target)
 		}
 	} else if !os.IsNotExist(err) {
-		return SkillInstallResult{}, fmt.Errorf("inspect skill target: %w", err)
+		return fmt.Errorf("inspect skill target: %w", err)
 	}
+	return fs.WalkDir(hopskill.Files, ".", func(path string, entry fs.DirEntry, walkErr error) error {
+		if walkErr != nil || path == "." {
+			return walkErr
+		}
+		destination := filepath.Join(target, filepath.FromSlash(path))
+		info, err := os.Lstat(destination)
+		if os.IsNotExist(err) {
+			return nil
+		}
+		if err != nil {
+			return fmt.Errorf("inspect skill destination %s: %w", destination, err)
+		}
+		if info.Mode()&os.ModeSymlink != 0 {
+			return fmt.Errorf("refusing to overwrite symlink %s", destination)
+		}
+		if entry.IsDir() && !info.IsDir() {
+			return fmt.Errorf("skill directory destination is not a directory: %s", destination)
+		}
+		if !entry.IsDir() && info.IsDir() {
+			return fmt.Errorf("skill file destination is a directory: %s", destination)
+		}
+		return nil
+	})
+}
+
+// preflightSkillAncestors finds the nearest existing path component before any
+// writes. This distinguishes an ordinary missing destination from a dangling
+// parent symlink, which filepath.EvalSymlinks otherwise reports as os.ErrNotExist.
+func preflightSkillAncestors(path string) error {
+	target := filepath.Clean(path)
+	current := target
+	for {
+		info, err := os.Lstat(current)
+		if err == nil {
+			if info.Mode()&os.ModeSymlink != 0 {
+				resolved, resolveErr := filepath.EvalSymlinks(current)
+				if resolveErr != nil {
+					return fmt.Errorf("resolve skill destination ancestor %s: %w", current, resolveErr)
+				}
+				resolvedInfo, statErr := os.Stat(resolved)
+				if statErr != nil {
+					return fmt.Errorf("inspect resolved skill destination ancestor %s: %w", current, statErr)
+				}
+				if current != target && !resolvedInfo.IsDir() {
+					return fmt.Errorf("skill destination ancestor is not a directory: %s", current)
+				}
+			} else if current != target && !info.IsDir() {
+				return fmt.Errorf("skill destination ancestor is not a directory: %s", current)
+			}
+			return nil
+		}
+		if !os.IsNotExist(err) {
+			return fmt.Errorf("inspect skill destination ancestor %s: %w", current, err)
+		}
+		parent := filepath.Dir(current)
+		if parent == current {
+			return nil
+		}
+		current = parent
+	}
+}
+
+func writeSkillTarget(target string) (SkillInstallResult, error) {
 	if err := os.MkdirAll(target, 0o755); err != nil {
 		return SkillInstallResult{}, fmt.Errorf("create skill target: %w", err)
 	}
 
-	result := SkillInstallResult{Path: target}
-	err = fs.WalkDir(hopskill.Files, ".", func(path string, entry fs.DirEntry, walkErr error) error {
+	result := SkillInstallResult{Path: target, Paths: []string{target}}
+	err := fs.WalkDir(hopskill.Files, ".", func(path string, entry fs.DirEntry, walkErr error) error {
 		if walkErr != nil {
 			return walkErr
 		}

internal/hop/skill_test.go

@@ -1,8 +1,10 @@
 package hop
 
 import (
+	"bytes"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 )
@@ -16,6 +18,9 @@ func TestInstallSkillBundle(t *testing.T) {
 	if result.Path != filepath.Join(base, "hop") {
 		t.Fatalf("skill path = %s", result.Path)
 	}
+	if len(result.Paths) != 1 || result.Paths[0] != result.Path {
+		t.Fatalf("skill paths = %#v, want only %s", result.Paths, result.Path)
+	}
 	wantFiles := []string{"SKILL.md", "agents/openai.yaml", "references/protocol.md"}
 	for _, relative := range wantFiles {
 		contents, err := os.ReadFile(filepath.Join(result.Path, relative))
@@ -31,7 +36,7 @@ func TestInstallSkillBundle(t *testing.T) {
 		t.Fatal(err)
 	}
 	if !strings.Contains(string(metadata), "allow_implicit_invocation: true") {
-		t.Fatal("installed skill does not permit Desktop implicit invocation")
+		t.Fatal("installed OpenAI metadata does not permit implicit invocation")
 	}
 	skill, err := os.ReadFile(filepath.Join(result.Path, "SKILL.md"))
 	if err != nil {
@@ -58,9 +63,149 @@ func TestInstallSkillBundle(t *testing.T) {
 	}
 }
 
+func TestInstallDefaultSkillsWritesSharedAndCodexBundles(t *testing.T) {
+	home := t.TempDir()
+	codexHome := filepath.Join(home, "codex-home")
+	t.Setenv("HOME", home)
+	t.Setenv("USERPROFILE", home)
+	t.Setenv("CODEX_HOME", codexHome)
+
+	result, err := InstallDefaultSkills(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	codexTarget := filepath.Join(codexHome, "skills", "hop")
+	sharedTarget := filepath.Join(home, ".agents", "skills", "hop")
+	if result.Path != codexTarget {
+		t.Fatalf("legacy primary path = %s, want %s", result.Path, codexTarget)
+	}
+	wantPaths := []string{codexTarget, sharedTarget}
+	if len(result.Paths) != len(wantPaths) {
+		t.Fatalf("default paths = %#v, want %#v", result.Paths, wantPaths)
+	}
+	for index, want := range wantPaths {
+		if result.Paths[index] != want {
+			t.Fatalf("default path %d = %s, want %s", index, result.Paths[index], want)
+		}
+	}
+	codexSkill, err := os.ReadFile(filepath.Join(codexTarget, "SKILL.md"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	sharedSkill, err := os.ReadFile(filepath.Join(sharedTarget, "SKILL.md"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(codexSkill, sharedSkill) {
+		t.Fatal("default skill bundles differ")
+	}
+}
+
+func TestInstallDefaultSkillsPreflightsAndDeduplicates(t *testing.T) {
+	t.Run("preflight", func(t *testing.T) {
+		home := t.TempDir()
+		codexHome := filepath.Join(home, "codex-home")
+		t.Setenv("HOME", home)
+		t.Setenv("USERPROFILE", home)
+		t.Setenv("CODEX_HOME", codexHome)
+		codexTarget := filepath.Join(codexHome, "skills", "hop")
+		if err := os.MkdirAll(codexTarget, 0o755); err != nil {
+			t.Fatal(err)
+		}
+		unknown := filepath.Join(codexTarget, "user-note.txt")
+		if err := os.WriteFile(unknown, []byte("keep me"), 0o644); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := InstallDefaultSkills(false); err == nil || !strings.Contains(err.Error(), "--force") {
+			t.Fatalf("default preflight error = %v", err)
+		}
+		sharedTarget := filepath.Join(home, ".agents", "skills", "hop")
+		if _, err := os.Stat(sharedTarget); !os.IsNotExist(err) {
+			t.Fatalf("partial shared install exists after preflight failure: %v", err)
+		}
+		if _, err := InstallDefaultSkills(true); err != nil {
+			t.Fatal(err)
+		}
+		if contents, err := os.ReadFile(unknown); err != nil || string(contents) != "keep me" {
+			t.Fatalf("force install removed unknown file: %q, %v", string(contents), err)
+		}
+	})
+
+	t.Run("nested symlink", func(t *testing.T) {
+		if runtime.GOOS == "windows" {
+			t.Skip("symlink creation requires additional privileges on Windows")
+		}
+		home := t.TempDir()
+		codexHome := filepath.Join(home, "codex-home")
+		t.Setenv("HOME", home)
+		t.Setenv("USERPROFILE", home)
+		t.Setenv("CODEX_HOME", codexHome)
+		sharedTarget := filepath.Join(home, ".agents", "skills", "hop")
+		if err := os.MkdirAll(sharedTarget, 0o755); err != nil {
+			t.Fatal(err)
+		}
+		outside := filepath.Join(home, "outside")
+		if err := os.WriteFile(outside, []byte("do not overwrite"), 0o644); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Symlink(outside, filepath.Join(sharedTarget, "SKILL.md")); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := InstallDefaultSkills(true); err == nil || !strings.Contains(err.Error(), "symlink") {
+			t.Fatalf("nested symlink preflight error = %v", err)
+		}
+		codexTarget := filepath.Join(codexHome, "skills", "hop")
+		if _, err := os.Stat(codexTarget); !os.IsNotExist(err) {
+			t.Fatalf("partial Codex install exists after nested preflight failure: %v", err)
+		}
+		if contents, err := os.ReadFile(outside); err != nil || string(contents) != "do not overwrite" {
+			t.Fatalf("nested symlink target changed: %q, %v", contents, err)
+		}
+	})
+
+	t.Run("dangling parent symlink", func(t *testing.T) {
+		if runtime.GOOS == "windows" {
+			t.Skip("symlink creation requires additional privileges on Windows")
+		}
+		home := t.TempDir()
+		codexHome := filepath.Join(home, "codex-home")
+		t.Setenv("HOME", home)
+		t.Setenv("USERPROFILE", home)
+		t.Setenv("CODEX_HOME", codexHome)
+		if err := os.Symlink(filepath.Join(home, "missing-agents-home"), filepath.Join(home, ".agents")); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := InstallDefaultSkills(true); err == nil || !strings.Contains(err.Error(), "ancestor") {
+			t.Fatalf("dangling parent preflight error = %v", err)
+		}
+		codexTarget := filepath.Join(codexHome, "skills", "hop")
+		if _, err := os.Stat(codexTarget); !os.IsNotExist(err) {
+			t.Fatalf("partial Codex install exists after dangling-parent failure: %v", err)
+		}
+	})
+
+	t.Run("deduplicate", func(t *testing.T) {
+		home := t.TempDir()
+		t.Setenv("HOME", home)
+		t.Setenv("USERPROFILE", home)
+		t.Setenv("CODEX_HOME", filepath.Join(home, ".agents"))
+		result, err := InstallDefaultSkills(false)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(result.Paths) != 1 {
+			t.Fatalf("aliased default paths were not deduplicated: %#v", result.Paths)
+		}
+	})
+}
+
 func TestSkillCLIWorksOutsideHopProject(t *testing.T) {
 	var stdout, stderr strings.Builder
-	base := t.TempDir()
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	t.Setenv("USERPROFILE", home)
+	t.Setenv("CODEX_HOME", filepath.Join(home, "codex-home"))
+	base := filepath.Join(home, "custom-skills")
 	code := RunCLI([]string{"skill", "install", "--path", base, "--json"}, &stdout, &stderr)
 	if code != 0 {
 		t.Fatalf("skill install exited %d: %s", code, stderr.String())
@@ -68,6 +213,14 @@ func TestSkillCLIWorksOutsideHopProject(t *testing.T) {
 	if !strings.Contains(stdout.String(), filepath.Join(base, "hop")) {
 		t.Fatalf("skill install JSON omitted target: %s", stdout.String())
 	}
+	for _, unexpected := range []string{
+		filepath.Join(home, ".agents", "skills", "hop"),
+		filepath.Join(home, "codex-home", "skills", "hop"),
+	} {
+		if _, err := os.Stat(unexpected); !os.IsNotExist(err) {
+			t.Fatalf("explicit --path also installed default target %s: %v", unexpected, err)
+		}
+	}
 	stdout.Reset()
 	stderr.Reset()
 	code = RunCLI([]string{"skill", "print"}, &stdout, &stderr)