diff --git a/internal/cli/config.go b/internal/cli/config.go index f38d406..dbac125 100644 --- a/internal/cli/config.go +++ b/internal/cli/config.go @@ -60,9 +60,11 @@ func runConfig(cmd *cobra.Command, local bool, explicitPath string) error { return fmt.Errorf("no configuration found at %s; run 'skill-sync init' to create one", configPath) } + // Agent directories live under the user home, not under the user config + // directory. These two lookups must stay separate. d := discovery.New() d.Getwd = getWd - d.UserHomeDir = func() (string, error) { return getUserConfig() } + d.UserHomeDir = getUserHome discovered, err := d.Scan() if err != nil { diff --git a/internal/cli/config_test.go b/internal/cli/config_test.go index b3f6282..caa122d 100644 --- a/internal/cli/config_test.go +++ b/internal/cli/config_test.go @@ -11,11 +11,19 @@ import ( func TestConfigCmd(t *testing.T) { origGetWd := getWd origGetUserConfig := getUserConfig + origGetUserHome := getUserHome defer func() { getWd = origGetWd getUserConfig = origGetUserConfig + getUserHome = origGetUserHome }() + // Pin the home seam to an empty directory so these cases never discover the + // developer's real agent directories. Global discovery has dedicated + // coverage in TestConfigDiscoversGlobalAgents. + emptyHome := t.TempDir() + getUserHome = func() (string, error) { return emptyHome, nil } + t.Run("default targets global config", func(t *testing.T) { tempDir := t.TempDir() getWd = func() (string, error) { return tempDir, nil } @@ -178,3 +186,80 @@ func TestConfigCmd(t *testing.T) { } }) } + +// TestConfigDiscoversGlobalAgents mirrors TestInitDiscoversGlobalAgents: the +// re-discovery path must resolve agent directories against the user home, not +// against the user config directory. +func TestConfigDiscoversGlobalAgents(t *testing.T) { + origGetWd := getWd + origGetUserConfig := getUserConfig + origGetUserHome := getUserHome + defer func() { + getWd = origGetWd + getUserConfig = origGetUserConfig + getUserHome = origGetUserHome + }() + + tests := []struct { + name string + globalDir string + want string + }{ + { + name: "discovers Pi which is global only", + globalDir: filepath.Join(".pi", "agent"), + want: ".pi/agent/skills", + }, + { + name: "discovers global Gemini CLI", + globalDir: ".gemini", + want: ".gemini/skills", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + homeDir := filepath.Join(root, "home") + configDir := filepath.Join(root, "xdg_config") + cwd := filepath.Join(root, "project") + + if err := os.MkdirAll(filepath.Join(homeDir, tt.globalDir), 0755); err != nil { + t.Fatalf("failed to seed agent dir: %v", err) + } + if err := os.MkdirAll(cwd, 0755); err != nil { + t.Fatalf("failed to seed cwd: %v", err) + } + + cfgPath := filepath.Join(configDir, "skill-sync", "skill-sync.yaml") + if err := os.MkdirAll(filepath.Dir(cfgPath), 0755); err != nil { + t.Fatalf("failed to seed config dir: %v", err) + } + if err := os.WriteFile(cfgPath, []byte("targets:\n - stale\n"), 0644); err != nil { + t.Fatalf("failed to seed config: %v", err) + } + + getWd = func() (string, error) { return cwd, nil } + getUserConfig = func() (string, error) { return configDir, nil } + getUserHome = func() (string, error) { return homeDir, nil } + + cmd := NewConfigCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetArgs([]string{}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + content, err := os.ReadFile(cfgPath) + if err != nil { + t.Fatalf("failed to read updated config: %v", err) + } + + if !strings.Contains(string(content), tt.want) { + t.Errorf("expected config to contain %q, got:\n%s", tt.want, string(content)) + } + }) + } +} diff --git a/internal/cli/init.go b/internal/cli/init.go index 2a0a6f3..e7f3519 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -37,6 +37,7 @@ func NewInitCmd() *cobra.Command { var ( getWd = os.Getwd getUserConfig = os.UserConfigDir + getUserHome = os.UserHomeDir stdinReader io.Reader = os.Stdin isTTY = func() bool { st, err := os.Stdin.Stat() @@ -83,9 +84,11 @@ func runInit(cmd *cobra.Command, local, force bool) error { } } + // Agent directories live under the user home, not under the user config + // directory. These two lookups must stay separate. d := discovery.New() d.Getwd = getWd - d.UserHomeDir = func() (string, error) { return getUserConfig() } + d.UserHomeDir = getUserHome targets, err := d.Scan() if err != nil { diff --git a/internal/cli/init_test.go b/internal/cli/init_test.go index 6f2625d..2bb4c02 100644 --- a/internal/cli/init_test.go +++ b/internal/cli/init_test.go @@ -11,15 +11,23 @@ import ( func TestInitCmd(t *testing.T) { origGetWd := getWd origGetUserConfig := getUserConfig + origGetUserHome := getUserHome origIsTTY := isTTY origStdin := stdinReader defer func() { getWd = origGetWd getUserConfig = origGetUserConfig + getUserHome = origGetUserHome isTTY = origIsTTY stdinReader = origStdin }() + // Pin the home seam to an empty directory so these cases never discover the + // developer's real agent directories. Global discovery has dedicated + // coverage in TestInitDiscoversGlobalAgents. + emptyHome := t.TempDir() + getUserHome = func() (string, error) { return emptyHome, nil } + t.Run("local init creates file in cwd with skills suffix", func(t *testing.T) { tempDir := t.TempDir() getWd = func() (string, error) { return tempDir, nil } @@ -192,3 +200,83 @@ func TestInitCmd(t *testing.T) { } }) } + +// TestInitDiscoversGlobalAgents pins the home/config seam separation. Agent +// directories live under the user home, not under the user config directory, so +// init must keep those two lookups independent. Pi is the canary: it has no +// local directory, so a collapsed seam makes it undiscoverable. +func TestInitDiscoversGlobalAgents(t *testing.T) { + origGetWd := getWd + origGetUserConfig := getUserConfig + origGetUserHome := getUserHome + origIsTTY := isTTY + defer func() { + getWd = origGetWd + getUserConfig = origGetUserConfig + getUserHome = origGetUserHome + isTTY = origIsTTY + }() + + tests := []struct { + name string + globalDir string + want string + }{ + { + name: "discovers Pi which is global only", + globalDir: filepath.Join(".pi", "agent"), + want: ".pi/agent/skills", + }, + { + name: "discovers global Claude Code", + globalDir: ".claude", + want: ".claude/skills", + }, + { + name: "discovers global OpenCode under .config", + globalDir: filepath.Join(".config", "opencode"), + want: ".config/opencode/skills", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + homeDir := filepath.Join(root, "home") + // The config directory is deliberately a sibling of home, never its + // parent, so a seam that confuses the two cannot accidentally pass. + configDir := filepath.Join(root, "xdg_config") + cwd := filepath.Join(root, "project") + + if err := os.MkdirAll(filepath.Join(homeDir, tt.globalDir), 0755); err != nil { + t.Fatalf("failed to seed agent dir: %v", err) + } + if err := os.MkdirAll(cwd, 0755); err != nil { + t.Fatalf("failed to seed cwd: %v", err) + } + + getWd = func() (string, error) { return cwd, nil } + getUserConfig = func() (string, error) { return configDir, nil } + getUserHome = func() (string, error) { return homeDir, nil } + isTTY = func() bool { return false } + + cmd := NewInitCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetArgs([]string{}) + + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + content, err := os.ReadFile(filepath.Join(configDir, "skill-sync", "skill-sync.yaml")) + if err != nil { + t.Fatalf("failed to read generated config: %v", err) + } + + if !strings.Contains(string(content), tt.want) { + t.Errorf("expected config to contain %q, got:\n%s", tt.want, string(content)) + } + }) + } +} diff --git a/odd/tasks/fix-global-agent-discovery.md b/odd/tasks/fix-global-agent-discovery.md new file mode 100644 index 0000000..b7b7d77 --- /dev/null +++ b/odd/tasks/fix-global-agent-discovery.md @@ -0,0 +1,82 @@ +# Fix global agent discovery in `init` and `config` + +## Problem + +`internal/cli/init.go:88` and `internal/cli/config.go:65` inject the user *config* +directory into the discovery *home* seam: + +```go +d.UserHomeDir = func() (string, error) { return getUserConfig() } +``` + +`getUserConfig` is `os.UserConfigDir` (`~/.config` on Linux), so `Discovery.Scan()` +resolves every global agent directory to `~/.config/.pi/agent`, `~/.config/.claude`, +etc. None of those exist, so no global agent is ever discovered. + +Pi is fully invisible because its `AgentConfig.LocalDir` is `""` +(`internal/agent/discovery/discovery.go:52`): it has only a global path. + +The TUI is unaffected — `internal/tui/program.go:71,86` call `discovery.New()` +without overriding the seam. + +## Root cause + +One package-level variable (`getUserConfig`) serves two distinct responsibilities: +locating `skill-sync.yaml` and locating the user home for agent discovery. + +## Fix + +Split the seam: keep `getUserConfig` for config location, add `getUserHome` +(`os.UserHomeDir`) for discovery. + +## Tasks + +- [x] 1. RED: test that `init` discovers a global-only agent (Pi) when home and + config directories differ. +- [x] 2. GREEN: add the `getUserHome` seam and wire it in `init.go`. +- [x] 3. RED: equivalent test for `config`. +- [x] 4. GREEN: wire `getUserHome` in `config.go`. +- [x] 5. Make existing `init`/`config` tests hermetic by stubbing `getUserHome`, + so they cannot read the developer's real home directory. +- [x] 6. Full suite + manual CLI verification against the real home. + +## Evidence + +**RED (task 1)** — `go test ./internal/cli/ -run TestInitDiscoversGlobalAgents`: +all three cases emitted an empty `targets:` list. + +**RED (task 3)** — `go test ./internal/cli/ -run TestConfigDiscoversGlobalAgents`: +both cases emitted `targets: []`. + +**Task 5 was not cosmetic.** After wiring the real home seam, the pre-existing +case `TestInitCmd/no_agents_found_prints_'no_skills_configured'` failed because it +started discovering the developer's actual `~/.claude`, `~/.gemini` and +`~/.pi/agent`. Those tests had been passing only because the broken seam pointed +them at a directory that never existed. Both suites now pin `getUserHome` to an +empty temporary directory. + +**GREEN** — `go test ./...`: all 14 packages pass. `go vet ./...` clean, +`gofmt -l internal/cli/` empty. + +**Manual** — `XDG_CONFIG_HOME=/tmp/pitest/cfg skill-sync init` against the real +home now produces: + +```yaml +targets: + - /home/eezzekl/.config/opencode/skills + - /home/eezzekl/.claude/skills + - /home/eezzekl/.gemini/skills + - /home/eezzekl/.copilot/skills + - /home/eezzekl/.pi/agent/skills +``` + +Previously this was an empty `targets:` list. + +## Notes + +`~/.pi/agent/skills` does not exist yet. `Discovery.Scan` matches on the agent +root (`~/.pi/agent`) and `init` appends `/skills`; `internal/sync/engine.go:43-56` +creates missing target directories, so the first `sync` materializes it. + +Pi project-local skills remain out of scope: `AgentConfig.LocalDir` for Pi is +`""` by design.