Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion internal/cli/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
85 changes: 85 additions & 0 deletions internal/cli/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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))
}
})
}
}
5 changes: 4 additions & 1 deletion internal/cli/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down
88 changes: 88 additions & 0 deletions internal/cli/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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))
}
})
}
}
82 changes: 82 additions & 0 deletions odd/tasks/fix-global-agent-discovery.md
Original file line number Diff line number Diff line change
@@ -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.
Loading