From 0bf86ddf2e028ebee4f11a1f289fa087f43b30a5 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Tue, 19 May 2026 18:35:25 +0530 Subject: [PATCH 01/23] feat: agent skills module --- internal/ai/skills/agent.go | 270 ++++++++++++++++++++++++++++++++++++ 1 file changed, 270 insertions(+) create mode 100644 internal/ai/skills/agent.go diff --git a/internal/ai/skills/agent.go b/internal/ai/skills/agent.go new file mode 100644 index 000000000..22bb6c440 --- /dev/null +++ b/internal/ai/skills/agent.go @@ -0,0 +1,270 @@ +package skills + +import ( + "os" + "os/exec" + "path/filepath" + "sync" +) + +// AgentConfig describes a single AI coding agent and where it reads skills from. +type AgentConfig struct { + ID string + DisplayName string + GlobalSkillsDir string + ProjectSkillsDir string + DetectMarkers []string // paths whose existence indicates the agent is installed (any one match is sufficient) + DetectBinaries []string // binary names to look up in PATH (any one match is sufficient) +} + +// IsInstalled reports whether the agent appears to be installed on this machine. +// It returns true if any marker path exists or any binary is found in PATH. +func (a AgentConfig) IsInstalled() bool { + for _, marker := range a.DetectMarkers { + if marker == "" { + continue + } + if _, err := os.Stat(marker); err == nil { + return true + } + } + for _, binary := range a.DetectBinaries { + if binary == "" { + continue + } + if _, err := exec.LookPath(binary); err == nil { + return true + } + } + return false +} + +// SupportedAgents is the full list of AI coding agents that support the agentskills spec. +var SupportedAgents []AgentConfig + +func init() { + home, _ := os.UserHomeDir() + if home == "" { + return + } + + SupportedAgents = []AgentConfig{ + { + ID: "claude-code", + DisplayName: "Claude Code", + GlobalSkillsDir: filepath.Join(home, ".claude", "skills"), + ProjectSkillsDir: filepath.Join(".claude", "skills"), + DetectMarkers: []string{filepath.Join(home, ".claude")}, + DetectBinaries: []string{"claude"}, + }, + { + ID: "cursor", + DisplayName: "Cursor", + GlobalSkillsDir: filepath.Join(home, ".cursor", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{filepath.Join(home, ".cursor")}, + DetectBinaries: []string{"cursor"}, + }, + { + ID: "github-copilot", + DisplayName: "GitHub Copilot", + GlobalSkillsDir: filepath.Join(home, ".copilot", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{ + filepath.Join(home, ".copilot"), + filepath.Join(home, ".config", "github-copilot"), + filepath.Join(home, ".config", "gh"), + }, + DetectBinaries: []string{"gh"}, + }, + { + ID: "gemini-cli", + DisplayName: "Gemini CLI", + GlobalSkillsDir: filepath.Join(home, ".gemini", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{filepath.Join(home, ".gemini")}, + DetectBinaries: []string{"gemini"}, + }, + { + ID: "roo", + DisplayName: "Roo Code", + GlobalSkillsDir: filepath.Join(home, ".roo", "skills"), + ProjectSkillsDir: filepath.Join(".roo", "skills"), + DetectMarkers: []string{filepath.Join(home, ".roo")}, + DetectBinaries: nil, + }, + { + ID: "goose", + DisplayName: "Goose", + GlobalSkillsDir: filepath.Join(home, ".config", "goose", "skills"), + ProjectSkillsDir: filepath.Join(".goose", "skills"), + DetectMarkers: []string{filepath.Join(home, ".config", "goose")}, + DetectBinaries: nil, + }, + { + ID: "opencode", + DisplayName: "OpenCode", + GlobalSkillsDir: filepath.Join(home, ".config", "opencode", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{filepath.Join(home, ".config", "opencode")}, + DetectBinaries: nil, + }, + { + ID: "codex", + DisplayName: "Codex (OpenAI)", + GlobalSkillsDir: filepath.Join(home, ".codex", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{os.Getenv("CODEX_HOME")}, + DetectBinaries: nil, + }, + { + ID: "windsurf", + DisplayName: "Windsurf", + GlobalSkillsDir: filepath.Join(home, ".windsurf", "skills"), + ProjectSkillsDir: filepath.Join(".windsurf", "skills"), + DetectMarkers: []string{filepath.Join(home, ".windsurf")}, + DetectBinaries: nil, + }, + { + ID: "continue", + DisplayName: "Continue", + GlobalSkillsDir: filepath.Join(home, ".continue", "skills"), + ProjectSkillsDir: filepath.Join(".continue", "skills"), + DetectMarkers: []string{filepath.Join(home, ".continue")}, + DetectBinaries: nil, + }, + { + ID: "amp", + DisplayName: "Amp", + GlobalSkillsDir: filepath.Join(home, ".config", "agents", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{filepath.Join(home, ".config", "amp")}, + DetectBinaries: nil, + }, + { + ID: "junie", + DisplayName: "Junie", + GlobalSkillsDir: filepath.Join(home, ".junie", "skills"), + ProjectSkillsDir: filepath.Join(".junie", "skills"), + DetectMarkers: []string{filepath.Join(home, ".junie")}, + DetectBinaries: nil, + }, + { + ID: "kiro-cli", + DisplayName: "Kiro CLI", + GlobalSkillsDir: filepath.Join(home, ".kiro", "skills"), + ProjectSkillsDir: filepath.Join(".kiro", "skills"), + DetectMarkers: []string{filepath.Join(home, ".kiro")}, + DetectBinaries: nil, + }, + { + ID: "cline", + DisplayName: "Cline", + GlobalSkillsDir: filepath.Join(home, ".agents", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{filepath.Join(home, ".cline")}, + DetectBinaries: nil, + }, + { + ID: "augment", + DisplayName: "Augment", + GlobalSkillsDir: filepath.Join(home, ".augment", "skills"), + ProjectSkillsDir: filepath.Join(".augment", "skills"), + DetectMarkers: []string{filepath.Join(home, ".augment")}, + DetectBinaries: nil, + }, + { + ID: "aider-desk", + DisplayName: "AiderDesk", + GlobalSkillsDir: filepath.Join(home, ".aider-desk", "skills"), + ProjectSkillsDir: filepath.Join(".aider-desk", "skills"), + DetectMarkers: []string{filepath.Join(home, ".aider-desk")}, + DetectBinaries: nil, + }, + { + ID: "warp", + DisplayName: "Warp", + GlobalSkillsDir: filepath.Join(home, ".config", "agents", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{filepath.Join(home, ".warp")}, + DetectBinaries: nil, + }, + { + ID: "openhands", + DisplayName: "OpenHands", + GlobalSkillsDir: filepath.Join(home, ".openhands", "skills"), + ProjectSkillsDir: filepath.Join(".openhands", "skills"), + DetectMarkers: nil, + DetectBinaries: nil, + }, + { + ID: "trae", + DisplayName: "Trae", + GlobalSkillsDir: filepath.Join(home, ".trae", "skills"), + ProjectSkillsDir: filepath.Join(".trae", "skills"), + DetectMarkers: nil, + DetectBinaries: nil, + }, + { + ID: "universal", + DisplayName: "Universal", + GlobalSkillsDir: filepath.Join(home, ".agents", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: nil, + DetectBinaries: nil, + }, + } +} + +var ( + detectedAgentsOnce sync.Once + detectedAgentsCache []AgentConfig +) + +// DetectedAgents returns the subset of SupportedAgents that are installed on this machine. +// The universal agent is always included. Result is cached after the first call. +func DetectedAgents() []AgentConfig { + detectedAgentsOnce.Do(func() { + for _, a := range SupportedAgents { + if a.ID == "universal" || a.IsInstalled() { + detectedAgentsCache = append(detectedAgentsCache, a) + } + } + }) + return detectedAgentsCache +} + +// FastPriorityAgents returns detected agents in the priority order used by --fast mode: +// claude-code, cursor, github-copilot, gemini-cli, then all other detected agents, +// with universal always appended last. +func FastPriorityAgents() []AgentConfig { + detected := DetectedAgents() + + priority := []string{"claude-code", "cursor", "github-copilot", "gemini-cli"} + byID := make(map[string]AgentConfig, len(detected)) + for _, a := range detected { + byID[a.ID] = a + } + + var result []AgentConfig + added := make(map[string]bool) + + for _, id := range priority { + if a, ok := byID[id]; ok && id != "universal" { + result = append(result, a) + added[id] = true + } + } + + for _, a := range detected { + if !added[a.ID] && a.ID != "universal" { + result = append(result, a) + } + } + + if a, ok := byID["universal"]; ok { + result = append(result, a) + } + + return result +} From 4f87731def7471c400a334e23b5e19ad65bdfb26 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Tue, 19 May 2026 20:01:28 +0530 Subject: [PATCH 02/23] fix: added test cases --- internal/ai/skills/agent.go | 4 +- internal/ai/skills/agent_test.go | 220 +++++++++++++++++++++++++++++++ 2 files changed, 222 insertions(+), 2 deletions(-) create mode 100644 internal/ai/skills/agent_test.go diff --git a/internal/ai/skills/agent.go b/internal/ai/skills/agent.go index 22bb6c440..76d170d20 100644 --- a/internal/ai/skills/agent.go +++ b/internal/ai/skills/agent.go @@ -217,8 +217,8 @@ func init() { } var ( - detectedAgentsOnce sync.Once - detectedAgentsCache []AgentConfig + detectedAgentsOnce sync.Once + detectedAgentsCache []AgentConfig ) // DetectedAgents returns the subset of SupportedAgents that are installed on this machine. diff --git a/internal/ai/skills/agent_test.go b/internal/ai/skills/agent_test.go new file mode 100644 index 000000000..92adc3f9c --- /dev/null +++ b/internal/ai/skills/agent_test.go @@ -0,0 +1,220 @@ +package skills + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIsInstalled(t *testing.T) { + t.Run("returns true when marker path exists", func(t *testing.T) { + dir := t.TempDir() + a := AgentConfig{DetectMarkers: []string{dir}} + assert.True(t, a.IsInstalled()) + }) + + t.Run("returns false when marker path does not exist", func(t *testing.T) { + a := AgentConfig{DetectMarkers: []string{"/this/path/definitely/does/not/exist/99999"}} + assert.False(t, a.IsInstalled()) + }) + + t.Run("skips empty marker strings", func(t *testing.T) { + a := AgentConfig{DetectMarkers: []string{"", "/also/does/not/exist/99999"}} + assert.False(t, a.IsInstalled()) + }) + + t.Run("returns true on first matching marker", func(t *testing.T) { + dir := t.TempDir() + a := AgentConfig{DetectMarkers: []string{"/does/not/exist", dir, "/also/does/not/exist"}} + assert.True(t, a.IsInstalled()) + }) + + t.Run("returns true when binary is found in PATH", func(t *testing.T) { + dir := t.TempDir() + bin := filepath.Join(dir, "auth0-test-sentinel") + require.NoError(t, os.WriteFile(bin, []byte("#!/bin/sh\n"), 0o755)) + t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + + a := AgentConfig{DetectBinaries: []string{"auth0-test-sentinel"}} + assert.True(t, a.IsInstalled()) + }) + + t.Run("returns false when binary is not found in PATH", func(t *testing.T) { + a := AgentConfig{DetectBinaries: []string{"this-binary-does-not-exist-99999"}} + assert.False(t, a.IsInstalled()) + }) + + t.Run("skips empty binary strings", func(t *testing.T) { + a := AgentConfig{DetectBinaries: []string{"", "also-does-not-exist-99999"}} + assert.False(t, a.IsInstalled()) + }) + + t.Run("returns false with no markers or binaries", func(t *testing.T) { + a := AgentConfig{} + assert.False(t, a.IsInstalled()) + }) + + t.Run("returns false with nil markers and binaries", func(t *testing.T) { + a := AgentConfig{DetectMarkers: nil, DetectBinaries: nil} + assert.False(t, a.IsInstalled()) + }) + + t.Run("binary check is tried when markers all miss", func(t *testing.T) { + dir := t.TempDir() + bin := filepath.Join(dir, "auth0-fallback-sentinel") + require.NoError(t, os.WriteFile(bin, []byte("#!/bin/sh\n"), 0o755)) + t.Setenv("PATH", dir+":"+os.Getenv("PATH")) + + a := AgentConfig{ + DetectMarkers: []string{"/does/not/exist/99999"}, + DetectBinaries: []string{"auth0-fallback-sentinel"}, + } + assert.True(t, a.IsInstalled()) + }) +} + +func TestSupportedAgents(t *testing.T) { + t.Run("is non-empty", func(t *testing.T) { + assert.NotEmpty(t, SupportedAgents) + }) + + t.Run("all agents have non-empty ID and DisplayName", func(t *testing.T) { + for _, a := range SupportedAgents { + assert.NotEmptyf(t, a.ID, "agent ID must not be empty") + assert.NotEmptyf(t, a.DisplayName, "agent %s DisplayName must not be empty", a.ID) + } + }) + + t.Run("all agents have non-empty skill dirs", func(t *testing.T) { + for _, a := range SupportedAgents { + assert.NotEmptyf(t, a.GlobalSkillsDir, "agent %s GlobalSkillsDir must not be empty", a.ID) + assert.NotEmptyf(t, a.ProjectSkillsDir, "agent %s ProjectSkillsDir must not be empty", a.ID) + } + }) + + t.Run("all agent IDs are unique", func(t *testing.T) { + seen := make(map[string]bool) + for _, a := range SupportedAgents { + assert.Falsef(t, seen[a.ID], "duplicate agent ID: %s", a.ID) + seen[a.ID] = true + } + }) + + t.Run("universal agent is present", func(t *testing.T) { + found := false + for _, a := range SupportedAgents { + if a.ID == "universal" { + found = true + break + } + } + assert.True(t, found) + }) + + t.Run("agents with no detection are detectable-never", func(t *testing.T) { + // openhands, trae, and universal have nil markers/binaries meaning IsInstalled always + // returns false for them; they are included through explicit ID checks instead. + noDetectIDs := []string{"openhands", "trae", "universal"} + byID := make(map[string]AgentConfig) + for _, a := range SupportedAgents { + byID[a.ID] = a + } + for _, id := range noDetectIDs { + a, ok := byID[id] + require.Truef(t, ok, "agent %s must be in SupportedAgents", id) + assert.Nilf(t, a.DetectMarkers, "agent %s should have nil DetectMarkers", id) + assert.Nilf(t, a.DetectBinaries, "agent %s should have nil DetectBinaries", id) + } + }) +} + +func TestDetectedAgents(t *testing.T) { + t.Run("always includes universal", func(t *testing.T) { + detected := DetectedAgents() + found := false + for _, a := range detected { + if a.ID == "universal" { + found = true + break + } + } + assert.True(t, found) + }) + + t.Run("returns consistent results on repeated calls", func(t *testing.T) { + first := DetectedAgents() + second := DetectedAgents() + assert.Equal(t, first, second) + }) + + t.Run("all returned agents come from SupportedAgents", func(t *testing.T) { + supported := make(map[string]bool, len(SupportedAgents)) + for _, a := range SupportedAgents { + supported[a.ID] = true + } + for _, a := range DetectedAgents() { + assert.Truef(t, supported[a.ID], "detected agent %s is not in SupportedAgents", a.ID) + } + }) +} + +func TestFastPriorityAgents(t *testing.T) { + t.Run("universal is always last", func(t *testing.T) { + result := FastPriorityAgents() + require.NotEmpty(t, result) + assert.Equal(t, "universal", result[len(result)-1].ID) + }) + + t.Run("no duplicates", func(t *testing.T) { + seen := make(map[string]bool) + for _, a := range FastPriorityAgents() { + assert.Falsef(t, seen[a.ID], "duplicate agent %s in FastPriorityAgents", a.ID) + seen[a.ID] = true + } + }) + + t.Run("contains all detected agents", func(t *testing.T) { + resultIDs := make(map[string]bool) + for _, a := range FastPriorityAgents() { + resultIDs[a.ID] = true + } + for _, a := range DetectedAgents() { + assert.Truef(t, resultIDs[a.ID], "detected agent %s missing from FastPriorityAgents", a.ID) + } + }) + + t.Run("priority agents appear before non-priority agents", func(t *testing.T) { + result := FastPriorityAgents() + prioritySet := map[string]bool{ + "claude-code": true, + "cursor": true, + "github-copilot": true, + "gemini-cli": true, + } + + lastPriorityIdx := -1 + firstNonPriorityIdx := -1 + for i, a := range result { + if a.ID == "universal" { + continue + } + if prioritySet[a.ID] { + lastPriorityIdx = i + } else if firstNonPriorityIdx == -1 { + firstNonPriorityIdx = i + } + } + + if lastPriorityIdx != -1 && firstNonPriorityIdx != -1 { + assert.Less(t, lastPriorityIdx, firstNonPriorityIdx, + "all priority agents must appear before any non-priority agent") + } + }) + + t.Run("result length equals detected agents count", func(t *testing.T) { + assert.Len(t, FastPriorityAgents(), len(DetectedAgents())) + }) +} From 9207d04aa870ac5c1e3e8479fffe72bf41254c15 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Tue, 19 May 2026 20:08:49 +0530 Subject: [PATCH 03/23] fix: lint fixes --- internal/ai/skills/agent.go | 4 ++-- internal/ai/skills/agent_test.go | 2 +- internal/auth0/quickstart.go | 3 ++- internal/cli/quickstart_detect.go | 12 ++++++------ 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/internal/ai/skills/agent.go b/internal/ai/skills/agent.go index 76d170d20..1f25f70f1 100644 --- a/internal/ai/skills/agent.go +++ b/internal/ai/skills/agent.go @@ -13,8 +13,8 @@ type AgentConfig struct { DisplayName string GlobalSkillsDir string ProjectSkillsDir string - DetectMarkers []string // paths whose existence indicates the agent is installed (any one match is sufficient) - DetectBinaries []string // binary names to look up in PATH (any one match is sufficient) + DetectMarkers []string // Paths whose existence indicates the agent is installed (any one match is sufficient). + DetectBinaries []string // Binary names to look up in PATH (any one match is sufficient). } // IsInstalled reports whether the agent appears to be installed on this machine. diff --git a/internal/ai/skills/agent_test.go b/internal/ai/skills/agent_test.go index 92adc3f9c..32224e594 100644 --- a/internal/ai/skills/agent_test.go +++ b/internal/ai/skills/agent_test.go @@ -115,7 +115,7 @@ func TestSupportedAgents(t *testing.T) { }) t.Run("agents with no detection are detectable-never", func(t *testing.T) { - // openhands, trae, and universal have nil markers/binaries meaning IsInstalled always + // Openhands, trae, and universal have nil markers/binaries meaning IsInstalled always // returns false for them; they are included through explicit ID checks instead. noDetectIDs := []string{"openhands", "trae", "universal"} byID := make(map[string]AgentConfig) diff --git a/internal/auth0/quickstart.go b/internal/auth0/quickstart.go index d8a83bcda..7e909f603 100644 --- a/internal/auth0/quickstart.go +++ b/internal/auth0/quickstart.go @@ -598,7 +598,8 @@ var QuickstartConfigs = map[string]AppConfig{ Callbacks: []string{DetectionSub}, AllowedLogoutURLs: []string{DetectionSub}, Name: DetectionSub, - CallbackPath: "/login/oauth2/code/okta", + // Okta-spring-boot-starter registers redirect under "oidc" registration ID. + CallbackPath: "/login/oauth2/code/oidc", }, Strategy: FileOutputStrategy{Path: "src/main/resources/application.yml", Format: "yaml"}, }, diff --git a/internal/cli/quickstart_detect.go b/internal/cli/quickstart_detect.go index d2f82d0b8..889097f30 100644 --- a/internal/cli/quickstart_detect.go +++ b/internal/cli/quickstart_detect.go @@ -35,7 +35,7 @@ func DetectProject(dir string) DetectionResult { // Read package.json deps early - needed for checks that must precede file-based signals. earlyDeps := readPackageJSONDeps(dir) - // -- 1. Manage.py (Django) — Checked before Ionic to prevent monorepo misdetection. + // -- 1. Manage.py (Django) — checked before Ionic to prevent monorepo misdetection. if fileExists(dir, "manage.py") { result.Framework = "django" result.Type = "regular" @@ -80,7 +80,7 @@ func DetectProject(dir string) DetectionResult { if data, ok := readFileContent(dir, "pubspec.yaml"); ok { if strings.Contains(data, "sdk: flutter") { result.Detected = true - // Android/iOS dirs signal native; flutter.web key signals web SPA; default native. + // Android/ios dirs signal native; flutter.web key signals web SPA; default native. switch { case dirExists(dir, "android") || dirExists(dir, "ios"): result.Framework = "flutter" @@ -99,7 +99,7 @@ func DetectProject(dir string) DetectionResult { } } - // -- 5. Composer.json (PHP) — Before vite.config to avoid Laravel misdetection. + // -- 5. Composer.json (PHP) — before vite.config to avoid Laravel misdetection. if data, ok := readFileContent(dir, "composer.json"); ok { result.BuildTool = "composer" result.Type = "regular" @@ -121,7 +121,7 @@ func DetectProject(dir string) DetectionResult { return result } - // -- 7. Nuxt.config.[ts|js] — Before vite.config (Nuxt uses Vite internally). + // -- 7. Nuxt.config.[ts|js] — before vite.config (Nuxt uses Vite internally). if fileExistsAny(dir, "nuxt.config.ts", "nuxt.config.js") { result.Framework = "nuxt" result.Type = "regular" @@ -155,7 +155,7 @@ func DetectProject(dir string) DetectionResult { return result } - // -- 10. Svelte.config.[js|ts] — Only label as sveltekit when @sveltejs/kit dep is confirmed. + // -- 10. Svelte.config.[js|ts] — only label as sveltekit when @sveltejs/kit dep is confirmed. if fileExistsAny(dir, "svelte.config.js", "svelte.config.ts") { framework := "sveltekit" appType := "regular" @@ -203,7 +203,7 @@ func DetectProject(dir string) DetectionResult { return result } - // -- 14. IOS Swift — .xcodeproj or Package.swift (Excludes Vapor server-side Swift). + // -- 14. IOS Swift — .xcodeproj or Package.swift (excludes Vapor server-side Swift). if hasXcodeprojDir(dir) || (fileExists(dir, "Package.swift") && !isVaporSwiftPackage(dir)) { result.Framework = "ios-swift" result.Type = "native" From b61277211fcfafe43daeb99c9a2c55808d20c581 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Wed, 20 May 2026 20:42:43 +0530 Subject: [PATCH 04/23] feat: download plugin module --- internal/ai/skills/download.go | 320 ++++++++++++++++++++++++++++ internal/ai/skills/download_test.go | 317 +++++++++++++++++++++++++++ 2 files changed, 637 insertions(+) create mode 100644 internal/ai/skills/download.go create mode 100644 internal/ai/skills/download_test.go diff --git a/internal/ai/skills/download.go b/internal/ai/skills/download.go new file mode 100644 index 000000000..e15092aff --- /dev/null +++ b/internal/ai/skills/download.go @@ -0,0 +1,320 @@ +package skills + +import ( + "archive/tar" + "archive/zip" + "compress/gzip" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "os/exec" + "path/filepath" + "strings" + "time" +) + +const ( + agentSkillsRepo = "https://github.com/auth0/agent-skills" + agentSkillsAPI = "https://api.github.com/repos/auth0/agent-skills/commits/" + pluginSubtreePath = "plugins/auth0" + skillsHTTPTimeout = 60 * time.Second + maxSkillsDownload = 100 * 1024 * 1024 // 100 MB +) + +var skillsHTTPClient = &http.Client{Timeout: skillsHTTPTimeout} + +// DownloadPlugin downloads the auth0 agent-skills plugin into targetDir using the best +// available strategy: git sparse-checkout > tar.gz > ZIP. Returns the commit SHA. +func DownloadPlugin(targetDir, ref string) (string, error) { + if ref == "" { + ref = "main" + } + + if _, err := exec.LookPath("git"); err == nil { + sha, err := downloadViaGit(targetDir, ref) + if err == nil { + return sha, nil + } + } + + sha, err := downloadViaTarGz(targetDir, ref) + if err == nil { + return sha, nil + } + + return downloadViaZip(targetDir, ref) +} + +// fetchCommitSHA fetches the latest commit SHA for ref from the GitHub API. +func fetchCommitSHA(ref string) (string, error) { + req, err := http.NewRequest(http.MethodGet, agentSkillsAPI+ref, nil) + if err != nil { + return "", err + } + req.Header.Set("Accept", "application/vnd.github.v3+json") + + resp, err := skillsHTTPClient.Do(req) + if err != nil { + return "", fmt.Errorf("github API request failed: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("github API returned status %d", resp.StatusCode) + } + + var payload struct { + SHA string `json:"sha"` + } + if err := json.NewDecoder(io.LimitReader(resp.Body, 1024*1024)).Decode(&payload); err != nil { + return "", fmt.Errorf("failed to decode github API response: %w", err) + } + if payload.SHA == "" { + return "", fmt.Errorf("github API returned empty SHA") + } + return payload.SHA, nil +} + +// downloadViaGit uses git sparse-checkout to download only plugins/auth0. +func downloadViaGit(targetDir, ref string) (string, error) { + tmpDir, err := os.MkdirTemp("", "auth0-agent-skills-*") + if err != nil { + return "", fmt.Errorf("failed to create temp dir: %w", err) + } + defer os.RemoveAll(tmpDir) + + run := func(args ...string) (string, error) { + cmd := exec.Command("git", args...) + cmd.Dir = tmpDir + out, err := cmd.CombinedOutput() + if err != nil { + return "", fmt.Errorf("git %s: %w\n%s", strings.Join(args, " "), err, out) + } + return strings.TrimSpace(string(out)), nil + } + + if _, err := run("clone", "--no-checkout", "--depth", "1", "--filter=blob:none", + agentSkillsRepo+".git", "."); err != nil { + return "", err + } + + if _, err := run("sparse-checkout", "set", pluginSubtreePath); err != nil { + return "", err + } + + if _, err := run("checkout"); err != nil { + return "", err + } + + sha, err := run("rev-parse", "HEAD") + if err != nil { + return "", err + } + + srcDir := filepath.Join(tmpDir, pluginSubtreePath) + if err := mergeDir(srcDir, targetDir); err != nil { + return "", err + } + + return sha, nil +} + +// fetchToTempFile downloads url into a new temp file and returns it open and seeked to the +// start, along with the number of bytes written. The caller is responsible for closing and +// removing the file. +func fetchToTempFile(url, pattern, label string) (*os.File, int64, error) { + resp, err := skillsHTTPClient.Get(url) + if err != nil { + return nil, 0, fmt.Errorf("%s download failed: %w", label, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, 0, fmt.Errorf("%s download returned status %d", label, resp.StatusCode) + } + + f, err := os.CreateTemp("", pattern) + if err != nil { + return nil, 0, err + } + + size, err := io.Copy(f, io.LimitReader(resp.Body, maxSkillsDownload)) + if err != nil { + _ = f.Close() + _ = os.Remove(f.Name()) + return nil, 0, fmt.Errorf("failed to save %s: %w", label, err) + } + + if _, err := f.Seek(0, io.SeekStart); err != nil { + _ = f.Close() + _ = os.Remove(f.Name()) + return nil, 0, err + } + + return f, size, nil +} + +// downloadViaTarGz downloads the archive from codeload.github.com and extracts the subtree. +func downloadViaTarGz(targetDir, ref string) (string, error) { + url := fmt.Sprintf("https://codeload.github.com/auth0/agent-skills/tar.gz/refs/heads/%s", ref) + f, _, err := fetchToTempFile(url, "auth0-agent-skills-*.tar.gz", "tar.gz") + if err != nil { + return "", err + } + defer os.Remove(f.Name()) + defer f.Close() + + prefix := fmt.Sprintf("auth0-agent-skills-%s/%s/", ref, pluginSubtreePath) + if err := extractTarGzSubtree(f, prefix, targetDir); err != nil { + return "", err + } + + return fetchCommitSHA(ref) +} + +// downloadViaZip downloads the ZIP archive from github.com and extracts the subtree. +func downloadViaZip(targetDir, ref string) (string, error) { + url := fmt.Sprintf("%s/archive/refs/heads/%s.zip", agentSkillsRepo, ref) + f, size, err := fetchToTempFile(url, "auth0-agent-skills-*.zip", "ZIP") + if err != nil { + return "", err + } + defer os.Remove(f.Name()) + defer f.Close() + + prefix := fmt.Sprintf("auth0-agent-skills-%s/%s/", ref, pluginSubtreePath) + if err := extractZipSubtree(f.Name(), size, prefix, targetDir); err != nil { + return "", err + } + + return fetchCommitSHA(ref) +} + +// extractTarGzSubtree reads a .tar.gz from r and copies entries whose name starts with +// prefix into destDir (stripping the prefix from the output path). +// extractEntry writes a single archive entry to destDir. isDir and mode describe the entry; +// open returns a reader for its content (ignored when isDir is true). The name is checked +// against prefix and any path-traversal attempt is rejected. +func extractEntry(name string, isDir bool, mode os.FileMode, open func() (io.ReadCloser, error), prefix, destDir string) error { + if !strings.HasPrefix(name, prefix) { + return nil + } + rel := strings.TrimPrefix(name, prefix) + if rel == "" { + return nil + } + dest := filepath.Join(destDir, filepath.FromSlash(rel)) + if !strings.HasPrefix(dest, filepath.Clean(destDir)+string(os.PathSeparator)) { + return fmt.Errorf("illegal path in archive: %s", name) + } + if isDir { + return os.MkdirAll(dest, 0o755) + } + if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { + return err + } + rc, err := open() + if err != nil { + return err + } + outFile, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, mode) + if err != nil { + _ = rc.Close() + return err + } + _, copyErr := io.Copy(outFile, rc) + _ = rc.Close() + _ = outFile.Close() + return copyErr +} + +// extractTarGzSubtree reads a .tar.gz from r and copies entries whose name starts with +// prefix into destDir (stripping the prefix from the output path). +func extractTarGzSubtree(r io.Reader, prefix, destDir string) error { + gz, err := gzip.NewReader(r) + if err != nil { + return fmt.Errorf("gzip open: %w", err) + } + defer gz.Close() + + tr := tar.NewReader(gz) + for { + hdr, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return fmt.Errorf("tar read: %w", err) + } + if err := extractEntry(hdr.Name, hdr.Typeflag == tar.TypeDir, hdr.FileInfo().Mode(), + func() (io.ReadCloser, error) { return io.NopCloser(tr), nil }, + prefix, destDir); err != nil { + return err + } + } + return nil +} + +// extractZipSubtree opens the ZIP at zipPath (with known size) and copies entries whose +// name starts with prefix into destDir (stripping the prefix). +func extractZipSubtree(zipPath string, size int64, prefix, destDir string) error { + // zip.NewReader needs an io.ReaderAt, so we re-open the file. + f, err := os.Open(zipPath) + if err != nil { + return err + } + defer f.Close() + + zr, err := zip.NewReader(f, size) + if err != nil { + return fmt.Errorf("zip open: %w", err) + } + + for _, entry := range zr.File { + if err := extractEntry(entry.Name, entry.FileInfo().IsDir(), entry.Mode(), + entry.Open, prefix, destDir); err != nil { + return err + } + } + return nil +} + +// mergeDir copies all files from src into dst, creating directories as needed. +func mergeDir(src, dst string) error { + return filepath.Walk(src, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + rel, err := filepath.Rel(src, path) + if err != nil { + return err + } + + target := filepath.Join(dst, rel) + + if info.IsDir() { + return os.MkdirAll(target, 0o755) + } + + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + return err + } + + in, err := os.Open(path) + if err != nil { + return err + } + out, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, info.Mode()) + if err != nil { + _ = in.Close() + return err + } + _, copyErr := io.Copy(out, in) + _ = in.Close() + _ = out.Close() + return copyErr + }) +} diff --git a/internal/ai/skills/download_test.go b/internal/ai/skills/download_test.go new file mode 100644 index 000000000..f65b99144 --- /dev/null +++ b/internal/ai/skills/download_test.go @@ -0,0 +1,317 @@ +package skills + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "encoding/json" + "errors" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// roundTripFunc lets a plain function satisfy http.RoundTripper. +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +// setHTTPClient replaces skillsHTTPClient for the duration of the test. +func setHTTPClient(t *testing.T, fn roundTripFunc) { + t.Helper() + orig := skillsHTTPClient + skillsHTTPClient = &http.Client{Transport: fn} + t.Cleanup(func() { skillsHTTPClient = orig }) +} + +// makeTarGz builds an in-memory .tar.gz from name→content pairs. +// A name ending in "/" is written as a directory entry. +func makeTarGz(t *testing.T, entries map[string]string) []byte { + t.Helper() + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + for name, content := range entries { + if strings.HasSuffix(name, "/") { + require.NoError(t, tw.WriteHeader(&tar.Header{Name: name, Typeflag: tar.TypeDir, Mode: 0o755})) + } else { + require.NoError(t, tw.WriteHeader(&tar.Header{Name: name, Typeflag: tar.TypeReg, Mode: 0o644, Size: int64(len(content))})) + _, err := tw.Write([]byte(content)) + require.NoError(t, err) + } + } + require.NoError(t, tw.Close()) + require.NoError(t, gw.Close()) + return buf.Bytes() +} + +// makeZip writes a ZIP archive to a temp file and returns its path and byte size. +func makeZip(t *testing.T, entries map[string]string) (path string, size int64) { + t.Helper() + f, err := os.CreateTemp("", "test-*.zip") + require.NoError(t, err) + t.Cleanup(func() { os.Remove(f.Name()) }) + + zw := zip.NewWriter(f) + for name, content := range entries { + w, err := zw.Create(name) + require.NoError(t, err) + _, err = w.Write([]byte(content)) + require.NoError(t, err) + } + require.NoError(t, zw.Close()) + size, err = f.Seek(0, io.SeekEnd) + require.NoError(t, err) + require.NoError(t, f.Close()) + return f.Name(), size +} + +func assertFileContent(t *testing.T, path, want string) { + t.Helper() + data, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, want, string(data)) +} + +// --- extractEntry --- + +func TestExtractEntry(t *testing.T) { + open := func(content string) func() (io.ReadCloser, error) { + return func() (io.ReadCloser, error) { + return io.NopCloser(strings.NewReader(content)), nil + } + } + + t.Run("skips entry not under prefix", func(t *testing.T) { + dest := t.TempDir() + require.NoError(t, extractEntry("other/file.txt", false, 0o644, open("x"), "prefix/", dest)) + entries, _ := os.ReadDir(dest) + assert.Empty(t, entries) + }) + + t.Run("skips root entry with empty rel", func(t *testing.T) { + dest := t.TempDir() + require.NoError(t, extractEntry("prefix/", false, 0o644, open("x"), "prefix/", dest)) + entries, _ := os.ReadDir(dest) + assert.Empty(t, entries) + }) + + t.Run("creates directory", func(t *testing.T) { + dest := t.TempDir() + require.NoError(t, extractEntry("prefix/subdir/", true, 0o755, nil, "prefix/", dest)) + info, err := os.Stat(filepath.Join(dest, "subdir")) + require.NoError(t, err) + assert.True(t, info.IsDir()) + }) + + t.Run("writes file content", func(t *testing.T) { + dest := t.TempDir() + require.NoError(t, extractEntry("prefix/file.txt", false, 0o644, open("hello"), "prefix/", dest)) + assertFileContent(t, filepath.Join(dest, "file.txt"), "hello") + }) + + t.Run("creates parent directories for nested file", func(t *testing.T) { + dest := t.TempDir() + require.NoError(t, extractEntry("prefix/a/b/c.txt", false, 0o644, open("nested"), "prefix/", dest)) + assertFileContent(t, filepath.Join(dest, "a", "b", "c.txt"), "nested") + }) + + t.Run("rejects path traversal", func(t *testing.T) { + dest := t.TempDir() + err := extractEntry("prefix/../../etc/passwd", false, 0o644, open("evil"), "prefix/", dest) + require.Error(t, err) + assert.Contains(t, err.Error(), "illegal path") + }) + + t.Run("propagates open error", func(t *testing.T) { + dest := t.TempDir() + boom := func() (io.ReadCloser, error) { return nil, errors.New("open failed") } + require.Error(t, extractEntry("prefix/file.txt", false, 0o644, boom, "prefix/", dest)) + }) +} + +// --- extractTarGzSubtree --- + +func TestExtractTarGzSubtree(t *testing.T) { + const prefix = "repo-main/plugins/auth0/" + + t.Run("extracts files under prefix and skips others", func(t *testing.T) { + data := makeTarGz(t, map[string]string{ + prefix + "skill-a/SKILL.md": "# skill-a", + prefix + "skill-b/SKILL.md": "# skill-b", + "unrelated/ignored.txt": "ignored", + }) + dest := t.TempDir() + require.NoError(t, extractTarGzSubtree(bytes.NewReader(data), prefix, dest)) + assertFileContent(t, filepath.Join(dest, "skill-a", "SKILL.md"), "# skill-a") + assertFileContent(t, filepath.Join(dest, "skill-b", "SKILL.md"), "# skill-b") + _, err := os.Stat(filepath.Join(dest, "unrelated")) + assert.True(t, os.IsNotExist(err)) + }) + + t.Run("creates directory entries", func(t *testing.T) { + data := makeTarGz(t, map[string]string{prefix + "skill-c/": ""}) + dest := t.TempDir() + require.NoError(t, extractTarGzSubtree(bytes.NewReader(data), prefix, dest)) + info, err := os.Stat(filepath.Join(dest, "skill-c")) + require.NoError(t, err) + assert.True(t, info.IsDir()) + }) + + t.Run("returns error on invalid gzip data", func(t *testing.T) { + err := extractTarGzSubtree(strings.NewReader("not gzip"), prefix, t.TempDir()) + require.Error(t, err) + }) +} + +// --- extractZipSubtree --- + +func TestExtractZipSubtree(t *testing.T) { + const prefix = "repo-main/plugins/auth0/" + + t.Run("extracts files under prefix and skips others", func(t *testing.T) { + zipPath, size := makeZip(t, map[string]string{ + prefix + "skill-x/SKILL.md": "# skill-x", + "unrelated/ignored.txt": "ignored", + }) + dest := t.TempDir() + require.NoError(t, extractZipSubtree(zipPath, size, prefix, dest)) + assertFileContent(t, filepath.Join(dest, "skill-x", "SKILL.md"), "# skill-x") + _, err := os.Stat(filepath.Join(dest, "unrelated")) + assert.True(t, os.IsNotExist(err)) + }) + + t.Run("returns error on invalid zip data", func(t *testing.T) { + f, err := os.CreateTemp("", "bad-*.zip") + require.NoError(t, err) + t.Cleanup(func() { os.Remove(f.Name()) }) + _, _ = f.WriteString("not a zip") + size, _ := f.Seek(0, io.SeekEnd) + require.NoError(t, f.Close()) + require.Error(t, extractZipSubtree(f.Name(), size, prefix, t.TempDir())) + }) + + t.Run("returns error when zip file does not exist", func(t *testing.T) { + require.Error(t, extractZipSubtree("/does/not/exist.zip", 0, prefix, t.TempDir())) + }) +} + +// --- mergeDir --- + +func TestMergeDir(t *testing.T) { + t.Run("copies flat files", func(t *testing.T) { + src, dst := t.TempDir(), t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(src, "a.txt"), []byte("aaa"), 0o644)) + require.NoError(t, mergeDir(src, dst)) + assertFileContent(t, filepath.Join(dst, "a.txt"), "aaa") + }) + + t.Run("copies nested files and creates subdirectories", func(t *testing.T) { + src, dst := t.TempDir(), t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(src, "sub", "deep"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(src, "sub", "deep", "b.txt"), []byte("bbb"), 0o644)) + require.NoError(t, mergeDir(src, dst)) + assertFileContent(t, filepath.Join(dst, "sub", "deep", "b.txt"), "bbb") + }) + + t.Run("overwrites existing destination files", func(t *testing.T) { + src, dst := t.TempDir(), t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(src, "f.txt"), []byte("new"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dst, "f.txt"), []byte("old"), 0o644)) + require.NoError(t, mergeDir(src, dst)) + assertFileContent(t, filepath.Join(dst, "f.txt"), "new") + }) +} + +// --- fetchToTempFile --- + +func TestFetchToTempFile(t *testing.T) { + t.Run("returns open seeked file and byte count on 200", func(t *testing.T) { + body := "file content" + setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body))}, nil + }) + f, size, err := fetchToTempFile("http://example.com/f", "test-*", "test") + require.NoError(t, err) + t.Cleanup(func() { f.Close(); os.Remove(f.Name()) }) + assert.Equal(t, int64(len(body)), size) + data, _ := io.ReadAll(f) + assert.Equal(t, body, string(data)) + }) + + t.Run("returns error on non-200 status", func(t *testing.T) { + setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader(""))}, nil + }) + _, _, err := fetchToTempFile("http://example.com/f", "test-*", "mylabel") + require.Error(t, err) + assert.Contains(t, err.Error(), "404") + }) + + t.Run("returns error on request failure", func(t *testing.T) { + setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { + return nil, errors.New("connection refused") + }) + _, _, err := fetchToTempFile("http://example.com/f", "test-*", "mylabel") + require.Error(t, err) + assert.Contains(t, err.Error(), "download failed") + }) +} + +// --- fetchCommitSHA --- + +func TestFetchCommitSHA(t *testing.T) { + shaResponse := func(sha string) roundTripFunc { + return func(_ *http.Request) (*http.Response, error) { + body, _ := json.Marshal(map[string]string{"sha": sha}) + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(body))}, nil + } + } + + t.Run("returns SHA from valid response", func(t *testing.T) { + setHTTPClient(t, shaResponse("abc123def456")) + sha, err := fetchCommitSHA("main") + require.NoError(t, err) + assert.Equal(t, "abc123def456", sha) + }) + + t.Run("returns error on non-200 status", func(t *testing.T) { + setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusForbidden, Body: io.NopCloser(strings.NewReader(""))}, nil + }) + _, err := fetchCommitSHA("main") + require.Error(t, err) + assert.Contains(t, err.Error(), "403") + }) + + t.Run("returns error when SHA field is empty", func(t *testing.T) { + setHTTPClient(t, shaResponse("")) + _, err := fetchCommitSHA("main") + require.Error(t, err) + assert.Contains(t, err.Error(), "empty SHA") + }) + + t.Run("returns error on invalid JSON body", func(t *testing.T) { + setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader("not json"))}, nil + }) + _, err := fetchCommitSHA("main") + require.Error(t, err) + }) + + t.Run("returns error on request failure", func(t *testing.T) { + setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { + return nil, errors.New("network error") + }) + _, err := fetchCommitSHA("main") + require.Error(t, err) + assert.Contains(t, err.Error(), "github API request failed") + }) +} From 3ba556cf92d1044c3380ca162c37f3fab869b135 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Thu, 21 May 2026 05:24:51 +0530 Subject: [PATCH 05/23] feat: add skills lock file support --- internal/ai/skills/lock.go | 50 ++++++++++ internal/ai/skills/lock_test.go | 156 ++++++++++++++++++++++++++++++++ 2 files changed, 206 insertions(+) create mode 100644 internal/ai/skills/lock.go create mode 100644 internal/ai/skills/lock_test.go diff --git a/internal/ai/skills/lock.go b/internal/ai/skills/lock.go new file mode 100644 index 000000000..8cf4f8a35 --- /dev/null +++ b/internal/ai/skills/lock.go @@ -0,0 +1,50 @@ +package skills + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "time" +) + +// SkillsLock records the installed state of the auth0 agent-skills plugin. +type SkillsLock struct { + Repo string `json:"repo"` + Ref string `json:"ref"` + CommitSHA string `json:"commitSHA"` + InstalledAt time.Time `json:"installedAt"` + UpdatedAt time.Time `json:"updatedAt"` + LastCheckedAt time.Time `json:"lastCheckedAt"` + Skills []string `json:"skills"` + Agents []string `json:"agents"` + Scope string `json:"scope"` // "global" or "local" +} + +// ReadLock reads the skills-lock.json at path. Returns nil, nil when the file does not exist. +func ReadLock(path string) (*SkillsLock, error) { + data, err := os.ReadFile(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + return nil, err + } + var lock SkillsLock + if err := json.Unmarshal(data, &lock); err != nil { + return nil, err + } + return &lock, nil +} + +// WriteLock serialises lock as JSON and writes it to path, creating parent directories as needed. +func WriteLock(path string, lock *SkillsLock) error { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + data, err := json.MarshalIndent(lock, "", " ") + if err != nil { + return err + } + return os.WriteFile(path, data, 0o644) +} diff --git a/internal/ai/skills/lock_test.go b/internal/ai/skills/lock_test.go new file mode 100644 index 000000000..fe1e6c0eb --- /dev/null +++ b/internal/ai/skills/lock_test.go @@ -0,0 +1,156 @@ +package skills + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReadLock(t *testing.T) { + t.Run("returns nil nil when file does not exist", func(t *testing.T) { + lock, err := ReadLock(filepath.Join(t.TempDir(), "skills-lock.json")) + require.NoError(t, err) + assert.Nil(t, lock) + }) + + t.Run("returns parsed lock for valid file", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "skills-lock.json") + content := `{ + "repo": "https://github.com/auth0/agent-skills.git", + "ref": "main", + "commitSHA": "abc123", + "installedAt": "2026-05-12T10:00:00Z", + "updatedAt": "2026-05-12T10:00:00Z", + "lastCheckedAt": "2026-05-12T11:00:00Z", + "skills": ["auth0-react", "auth0-nextjs"], + "agents": ["claude-code"], + "scope": "global" +}` + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) + + lock, err := ReadLock(path) + require.NoError(t, err) + require.NotNil(t, lock) + assert.Equal(t, "https://github.com/auth0/agent-skills.git", lock.Repo) + assert.Equal(t, "main", lock.Ref) + assert.Equal(t, "abc123", lock.CommitSHA) + assert.Equal(t, []string{"auth0-react", "auth0-nextjs"}, lock.Skills) + assert.Equal(t, []string{"claude-code"}, lock.Agents) + assert.Equal(t, "global", lock.Scope) + }) + + t.Run("returns error for invalid JSON", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "skills-lock.json") + require.NoError(t, os.WriteFile(path, []byte("not json"), 0o644)) + + _, err := ReadLock(path) + require.Error(t, err) + }) + + t.Run("returns error on unreadable file", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "skills-lock.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o000)) + t.Cleanup(func() { os.Chmod(path, 0o644) }) + + if os.Getuid() == 0 { + t.Skip("root bypasses file permissions") + } + _, err := ReadLock(path) + require.Error(t, err) + }) +} + +func TestWriteLock(t *testing.T) { + now := time.Date(2026, 5, 12, 10, 0, 0, 0, time.UTC) + + t.Run("creates file with correct content", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "skills-lock.json") + + lock := &SkillsLock{ + Repo: "https://github.com/auth0/agent-skills.git", + Ref: "main", + CommitSHA: "deadbeef", + InstalledAt: now, + UpdatedAt: now, + LastCheckedAt: now, + Skills: []string{"auth0-react"}, + Agents: []string{"cursor"}, + Scope: "local", + } + require.NoError(t, WriteLock(path, lock)) + + got, err := ReadLock(path) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, lock.Repo, got.Repo) + assert.Equal(t, lock.CommitSHA, got.CommitSHA) + assert.Equal(t, lock.Skills, got.Skills) + assert.Equal(t, lock.Scope, got.Scope) + assert.Equal(t, lock.InstalledAt.UTC(), got.InstalledAt.UTC()) + assert.Equal(t, lock.LastCheckedAt.UTC(), got.LastCheckedAt.UTC()) + }) + + t.Run("creates parent directories when they do not exist", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "nested", "deep", "skills-lock.json") + + require.NoError(t, WriteLock(path, &SkillsLock{Scope: "global"})) + + got, err := ReadLock(path) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, "global", got.Scope) + }) + + t.Run("overwrites existing lock file", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "skills-lock.json") + + require.NoError(t, WriteLock(path, &SkillsLock{CommitSHA: "first"})) + require.NoError(t, WriteLock(path, &SkillsLock{CommitSHA: "second"})) + + got, err := ReadLock(path) + require.NoError(t, err) + assert.Equal(t, "second", got.CommitSHA) + }) + + t.Run("roundtrip preserves all fields", func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "skills-lock.json") + + original := &SkillsLock{ + Repo: "https://github.com/auth0/agent-skills.git", + Ref: "v1.2.3", + CommitSHA: "cafebabe", + InstalledAt: now, + UpdatedAt: now.Add(time.Hour), + LastCheckedAt: now.Add(2 * time.Hour), + Skills: []string{"auth0-react", "auth0-nextjs", "auth0-vue"}, + Agents: []string{"claude-code", "cursor", "gemini-cli"}, + Scope: "global", + } + + require.NoError(t, WriteLock(path, original)) + got, err := ReadLock(path) + require.NoError(t, err) + require.NotNil(t, got) + + assert.Equal(t, original.Repo, got.Repo) + assert.Equal(t, original.Ref, got.Ref) + assert.Equal(t, original.CommitSHA, got.CommitSHA) + assert.Equal(t, original.InstalledAt.UTC(), got.InstalledAt.UTC()) + assert.Equal(t, original.UpdatedAt.UTC(), got.UpdatedAt.UTC()) + assert.Equal(t, original.LastCheckedAt.UTC(), got.LastCheckedAt.UTC()) + assert.Equal(t, original.Skills, got.Skills) + assert.Equal(t, original.Agents, got.Agents) + assert.Equal(t, original.Scope, got.Scope) + }) +} From c05b410b1e48240bfae26b215fdb229f92faf766 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Thu, 21 May 2026 05:53:50 +0530 Subject: [PATCH 06/23] feat: skills internal download and lock --- internal/ai/skills/download.go | 8 +++----- internal/ai/skills/download_test.go | 12 ++++++------ internal/ai/skills/lock.go | 12 ++++++------ internal/ai/skills/lock_test.go | 10 +++++----- 4 files changed, 20 insertions(+), 22 deletions(-) diff --git a/internal/ai/skills/download.go b/internal/ai/skills/download.go index e15092aff..fae60ea69 100644 --- a/internal/ai/skills/download.go +++ b/internal/ai/skills/download.go @@ -20,7 +20,7 @@ const ( agentSkillsAPI = "https://api.github.com/repos/auth0/agent-skills/commits/" pluginSubtreePath = "plugins/auth0" skillsHTTPTimeout = 60 * time.Second - maxSkillsDownload = 100 * 1024 * 1024 // 100 MB + maxSkillsDownload = 100 * 1024 * 1024 // 100 MB. ) var skillsHTTPClient = &http.Client{Timeout: skillsHTTPTimeout} @@ -192,9 +192,7 @@ func downloadViaZip(targetDir, ref string) (string, error) { return fetchCommitSHA(ref) } -// extractTarGzSubtree reads a .tar.gz from r and copies entries whose name starts with -// prefix into destDir (stripping the prefix from the output path). -// extractEntry writes a single archive entry to destDir. isDir and mode describe the entry; +// ExtractEntry writes a single archive entry to destDir. IsDir and mode describe the entry; // open returns a reader for its content (ignored when isDir is true). The name is checked // against prefix and any path-traversal attempt is rejected. func extractEntry(name string, isDir bool, mode os.FileMode, open func() (io.ReadCloser, error), prefix, destDir string) error { @@ -260,7 +258,7 @@ func extractTarGzSubtree(r io.Reader, prefix, destDir string) error { // extractZipSubtree opens the ZIP at zipPath (with known size) and copies entries whose // name starts with prefix into destDir (stripping the prefix). func extractZipSubtree(zipPath string, size int64, prefix, destDir string) error { - // zip.NewReader needs an io.ReaderAt, so we re-open the file. + // Zip.NewReader needs an io.ReaderAt, so we re-open the file. f, err := os.Open(zipPath) if err != nil { return err diff --git a/internal/ai/skills/download_test.go b/internal/ai/skills/download_test.go index f65b99144..34a1f2e8f 100644 --- a/internal/ai/skills/download_test.go +++ b/internal/ai/skills/download_test.go @@ -80,7 +80,7 @@ func assertFileContent(t *testing.T, path, want string) { assert.Equal(t, want, string(data)) } -// --- extractEntry --- +// --- extractEntry ---. func TestExtractEntry(t *testing.T) { open := func(content string) func() (io.ReadCloser, error) { @@ -137,7 +137,7 @@ func TestExtractEntry(t *testing.T) { }) } -// --- extractTarGzSubtree --- +// --- extractTarGzSubtree ---. func TestExtractTarGzSubtree(t *testing.T) { const prefix = "repo-main/plugins/auth0/" @@ -171,7 +171,7 @@ func TestExtractTarGzSubtree(t *testing.T) { }) } -// --- extractZipSubtree --- +// --- extractZipSubtree ---. func TestExtractZipSubtree(t *testing.T) { const prefix = "repo-main/plugins/auth0/" @@ -203,7 +203,7 @@ func TestExtractZipSubtree(t *testing.T) { }) } -// --- mergeDir --- +// --- mergeDir ---. func TestMergeDir(t *testing.T) { t.Run("copies flat files", func(t *testing.T) { @@ -230,7 +230,7 @@ func TestMergeDir(t *testing.T) { }) } -// --- fetchToTempFile --- +// --- fetchToTempFile ---. func TestFetchToTempFile(t *testing.T) { t.Run("returns open seeked file and byte count on 200", func(t *testing.T) { @@ -265,7 +265,7 @@ func TestFetchToTempFile(t *testing.T) { }) } -// --- fetchCommitSHA --- +// --- fetchCommitSHA ---. func TestFetchCommitSHA(t *testing.T) { shaResponse := func(sha string) roundTripFunc { diff --git a/internal/ai/skills/lock.go b/internal/ai/skills/lock.go index 8cf4f8a35..6dbb61a3b 100644 --- a/internal/ai/skills/lock.go +++ b/internal/ai/skills/lock.go @@ -8,8 +8,8 @@ import ( "time" ) -// SkillsLock records the installed state of the auth0 agent-skills plugin. -type SkillsLock struct { +// Lock records the installed state of the auth0 agent-skills plugin. +type Lock struct { Repo string `json:"repo"` Ref string `json:"ref"` CommitSHA string `json:"commitSHA"` @@ -18,11 +18,11 @@ type SkillsLock struct { LastCheckedAt time.Time `json:"lastCheckedAt"` Skills []string `json:"skills"` Agents []string `json:"agents"` - Scope string `json:"scope"` // "global" or "local" + Scope string `json:"scope"` // "global" or "local". } // ReadLock reads the skills-lock.json at path. Returns nil, nil when the file does not exist. -func ReadLock(path string) (*SkillsLock, error) { +func ReadLock(path string) (*Lock, error) { data, err := os.ReadFile(path) if err != nil { if errors.Is(err, os.ErrNotExist) { @@ -30,7 +30,7 @@ func ReadLock(path string) (*SkillsLock, error) { } return nil, err } - var lock SkillsLock + var lock Lock if err := json.Unmarshal(data, &lock); err != nil { return nil, err } @@ -38,7 +38,7 @@ func ReadLock(path string) (*SkillsLock, error) { } // WriteLock serialises lock as JSON and writes it to path, creating parent directories as needed. -func WriteLock(path string, lock *SkillsLock) error { +func WriteLock(path string, lock *Lock) error { if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return err } diff --git a/internal/ai/skills/lock_test.go b/internal/ai/skills/lock_test.go index fe1e6c0eb..5c50213aa 100644 --- a/internal/ai/skills/lock_test.go +++ b/internal/ai/skills/lock_test.go @@ -74,7 +74,7 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "skills-lock.json") - lock := &SkillsLock{ + lock := &Lock{ Repo: "https://github.com/auth0/agent-skills.git", Ref: "main", CommitSHA: "deadbeef", @@ -102,7 +102,7 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "nested", "deep", "skills-lock.json") - require.NoError(t, WriteLock(path, &SkillsLock{Scope: "global"})) + require.NoError(t, WriteLock(path, &Lock{Scope: "global"})) got, err := ReadLock(path) require.NoError(t, err) @@ -114,8 +114,8 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "skills-lock.json") - require.NoError(t, WriteLock(path, &SkillsLock{CommitSHA: "first"})) - require.NoError(t, WriteLock(path, &SkillsLock{CommitSHA: "second"})) + require.NoError(t, WriteLock(path, &Lock{CommitSHA: "first"})) + require.NoError(t, WriteLock(path, &Lock{CommitSHA: "second"})) got, err := ReadLock(path) require.NoError(t, err) @@ -126,7 +126,7 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "skills-lock.json") - original := &SkillsLock{ + original := &Lock{ Repo: "https://github.com/auth0/agent-skills.git", Ref: "v1.2.3", CommitSHA: "cafebabe", From 367802dea7c50097fde22339f9d6bdd3e43c699b Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Fri, 22 May 2026 16:42:45 +0530 Subject: [PATCH 07/23] feat: skills directory meta parsing for interactive display while installspecific skills for specific agents --- internal/ai/skills/skill_meta.go | 63 +++++++++++ internal/ai/skills/skill_meta_test.go | 144 ++++++++++++++++++++++++++ 2 files changed, 207 insertions(+) create mode 100644 internal/ai/skills/skill_meta.go create mode 100644 internal/ai/skills/skill_meta_test.go diff --git a/internal/ai/skills/skill_meta.go b/internal/ai/skills/skill_meta.go new file mode 100644 index 000000000..ddf757e74 --- /dev/null +++ b/internal/ai/skills/skill_meta.go @@ -0,0 +1,63 @@ +package skills + +import ( + "os" + "path/filepath" + "sort" + "strings" + + "gopkg.in/yaml.v3" +) + +// SkillMeta holds the name and description extracted from a SKILL.md frontmatter. +type SkillMeta struct { + Name string `yaml:"name"` + Description string `yaml:"description"` +} + +// ParseSkillMeta reads SKILL.md from skillDir and extracts the YAML frontmatter. +// Returns an empty SkillMeta (no error) when the file has no valid frontmatter delimiters. +func ParseSkillMeta(skillDir string) (SkillMeta, error) { + data, err := os.ReadFile(filepath.Join(skillDir, "SKILL.md")) + if err != nil { + return SkillMeta{}, err + } + + parts := strings.SplitN(string(data), "---", 3) + if len(parts) < 3 { + return SkillMeta{}, nil + } + + var meta SkillMeta + if err := yaml.Unmarshal([]byte(parts[1]), &meta); err != nil { + return SkillMeta{}, err + } + return meta, nil +} + +// ListAvailableSkills walks pluginSkillsDir and returns SkillMeta for every +// subdirectory that contains a valid SKILL.md, sorted alphabetically by name. +func ListAvailableSkills(pluginSkillsDir string) ([]SkillMeta, error) { + entries, err := os.ReadDir(pluginSkillsDir) + if err != nil { + return nil, err + } + + var skills []SkillMeta + for _, entry := range entries { + if !entry.IsDir() { + continue + } + meta, err := ParseSkillMeta(filepath.Join(pluginSkillsDir, entry.Name())) + if err != nil { + continue + } + skills = append(skills, meta) + } + + sort.Slice(skills, func(i, j int) bool { + return skills[i].Name < skills[j].Name + }) + + return skills, nil +} diff --git a/internal/ai/skills/skill_meta_test.go b/internal/ai/skills/skill_meta_test.go new file mode 100644 index 000000000..03b4c0bb1 --- /dev/null +++ b/internal/ai/skills/skill_meta_test.go @@ -0,0 +1,144 @@ +package skills + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeSkillMD(t *testing.T, dir, name, description, body string) { + t.Helper() + skillDir := filepath.Join(dir, name) + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + content := "---\nname: " + name + "\ndescription: " + description + "\n---\n" + body + require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644)) +} + +func TestParseSkillMeta(t *testing.T) { + t.Run("parses name and description from valid frontmatter", func(t *testing.T) { + dir := t.TempDir() + skillDir := filepath.Join(dir, "auth0-react") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + content := "---\nname: auth0-react\ndescription: Auth0 React integration\n---\n\n# Auth0 React\n" + require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644)) + + meta, err := ParseSkillMeta(skillDir) + require.NoError(t, err) + assert.Equal(t, "auth0-react", meta.Name) + assert.Equal(t, "Auth0 React integration", meta.Description) + }) + + t.Run("returns error when SKILL.md does not exist", func(t *testing.T) { + _, err := ParseSkillMeta(t.TempDir()) + require.Error(t, err) + }) + + t.Run("returns empty meta when no frontmatter delimiters", func(t *testing.T) { + dir := t.TempDir() + skillDir := filepath.Join(dir, "no-frontmatter") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# Just a heading\n"), 0o644)) + + meta, err := ParseSkillMeta(skillDir) + require.NoError(t, err) + assert.Equal(t, SkillMeta{}, meta) + }) + + t.Run("returns empty meta when only one delimiter present", func(t *testing.T) { + dir := t.TempDir() + skillDir := filepath.Join(dir, "partial") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\nname: foo\n"), 0o644)) + + meta, err := ParseSkillMeta(skillDir) + require.NoError(t, err) + assert.Equal(t, SkillMeta{}, meta) + }) + + t.Run("returns error for invalid YAML in frontmatter", func(t *testing.T) { + dir := t.TempDir() + skillDir := filepath.Join(dir, "bad-yaml") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + // Indentation error creates invalid YAML + content := "---\nname: foo\n bad: indent: here\n---\n" + require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644)) + + _, err := ParseSkillMeta(skillDir) + require.Error(t, err) + }) + + t.Run("only name is populated when description is absent", func(t *testing.T) { + dir := t.TempDir() + skillDir := filepath.Join(dir, "name-only") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + content := "---\nname: auth0-vue\n---\n" + require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644)) + + meta, err := ParseSkillMeta(skillDir) + require.NoError(t, err) + assert.Equal(t, "auth0-vue", meta.Name) + assert.Equal(t, "", meta.Description) + }) +} + +func TestListAvailableSkills(t *testing.T) { + t.Run("returns error when directory does not exist", func(t *testing.T) { + _, err := ListAvailableSkills(filepath.Join(t.TempDir(), "nonexistent")) + require.Error(t, err) + }) + + t.Run("returns empty slice for empty directory", func(t *testing.T) { + skills, err := ListAvailableSkills(t.TempDir()) + require.NoError(t, err) + assert.Empty(t, skills) + }) + + t.Run("returns sorted skills from multiple subdirectories", func(t *testing.T) { + dir := t.TempDir() + writeSkillMD(t, dir, "auth0-vue", "Auth0 Vue integration", "") + writeSkillMD(t, dir, "auth0-nextjs", "Auth0 Next.js integration", "") + writeSkillMD(t, dir, "auth0-react", "Auth0 React integration", "") + + skills, err := ListAvailableSkills(dir) + require.NoError(t, err) + require.Len(t, skills, 3) + assert.Equal(t, "auth0-nextjs", skills[0].Name) + assert.Equal(t, "auth0-react", skills[1].Name) + assert.Equal(t, "auth0-vue", skills[2].Name) + }) + + t.Run("skips non-directory entries", func(t *testing.T) { + dir := t.TempDir() + writeSkillMD(t, dir, "auth0-react", "Auth0 React integration", "") + require.NoError(t, os.WriteFile(filepath.Join(dir, "README.md"), []byte("# readme"), 0o644)) + + skills, err := ListAvailableSkills(dir) + require.NoError(t, err) + require.Len(t, skills, 1) + assert.Equal(t, "auth0-react", skills[0].Name) + }) + + t.Run("skips subdirectories without SKILL.md", func(t *testing.T) { + dir := t.TempDir() + writeSkillMD(t, dir, "auth0-react", "Auth0 React integration", "") + require.NoError(t, os.MkdirAll(filepath.Join(dir, "not-a-skill"), 0o755)) + + skills, err := ListAvailableSkills(dir) + require.NoError(t, err) + require.Len(t, skills, 1) + assert.Equal(t, "auth0-react", skills[0].Name) + }) + + t.Run("description is populated from frontmatter", func(t *testing.T) { + dir := t.TempDir() + writeSkillMD(t, dir, "auth0-nextjs", "Next.js with Auth0", "") + + skills, err := ListAvailableSkills(dir) + require.NoError(t, err) + require.Len(t, skills, 1) + assert.Equal(t, "Next.js with Auth0", skills[0].Description) + }) +} From b6ddb7761f266f5f2735da7e43a8a17f9a63e6f0 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Mon, 1 Jun 2026 21:22:49 +0530 Subject: [PATCH 08/23] fix: added review fixes for agent.go --- internal/ai/skills/agent.go | 112 ++++++++++++++-------- internal/ai/skills/agent_test.go | 155 ++++++++++++++++++++++++++++++- 2 files changed, 223 insertions(+), 44 deletions(-) diff --git a/internal/ai/skills/agent.go b/internal/ai/skills/agent.go index 1f25f70f1..c6da6ecff 100644 --- a/internal/ai/skills/agent.go +++ b/internal/ai/skills/agent.go @@ -7,18 +7,26 @@ import ( "sync" ) -// AgentConfig describes a single AI coding agent and where it reads skills from. type AgentConfig struct { - ID string - DisplayName string - GlobalSkillsDir string - ProjectSkillsDir string - DetectMarkers []string // Paths whose existence indicates the agent is installed (any one match is sufficient). - DetectBinaries []string // Binary names to look up in PATH (any one match is sufficient). + ID string + DisplayName string + GlobalSkillsDir string + GlobalSkillsDirEnvVar string + ProjectSkillsDir string + DetectMarkers []string + DetectMarkerEnvVars []string + DetectBinaries []string +} + +func (a AgentConfig) ResolvedGlobalSkillsDir() string { + if a.GlobalSkillsDirEnvVar != "" { + if v := os.Getenv(a.GlobalSkillsDirEnvVar); v != "" { + return filepath.Join(v, "skills") + } + } + return a.GlobalSkillsDir } -// IsInstalled reports whether the agent appears to be installed on this machine. -// It returns true if any marker path exists or any binary is found in PATH. func (a AgentConfig) IsInstalled() bool { for _, marker := range a.DetectMarkers { if marker == "" { @@ -28,6 +36,16 @@ func (a AgentConfig) IsInstalled() bool { return true } } + for _, envVar := range a.DetectMarkerEnvVars { + if envVar == "" { + continue + } + if v := os.Getenv(envVar); v != "" { + if _, err := os.Stat(v); err == nil { + return true + } + } + } for _, binary := range a.DetectBinaries { if binary == "" { continue @@ -39,12 +57,14 @@ func (a AgentConfig) IsInstalled() bool { return false } -// SupportedAgents is the full list of AI coding agents that support the agentskills spec. var SupportedAgents []AgentConfig func init() { home, _ := os.UserHomeDir() if home == "" { + SupportedAgents = []AgentConfig{ + {ID: "universal", DisplayName: "Universal", ProjectSkillsDir: filepath.Join(".agents", "skills")}, + } return } @@ -73,9 +93,8 @@ func init() { DetectMarkers: []string{ filepath.Join(home, ".copilot"), filepath.Join(home, ".config", "github-copilot"), - filepath.Join(home, ".config", "gh"), }, - DetectBinaries: []string{"gh"}, + DetectBinaries: []string{"code"}, }, { ID: "gemini-cli", @@ -85,13 +104,19 @@ func init() { DetectMarkers: []string{filepath.Join(home, ".gemini")}, DetectBinaries: []string{"gemini"}, }, + { + ID: "antigravity", + DisplayName: "Antigravity", + GlobalSkillsDir: filepath.Join(home, ".gemini", "antigravity", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{filepath.Join(home, ".gemini", "antigravity")}, + }, { ID: "roo", DisplayName: "Roo Code", GlobalSkillsDir: filepath.Join(home, ".roo", "skills"), ProjectSkillsDir: filepath.Join(".roo", "skills"), DetectMarkers: []string{filepath.Join(home, ".roo")}, - DetectBinaries: nil, }, { ID: "goose", @@ -99,7 +124,6 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".config", "goose", "skills"), ProjectSkillsDir: filepath.Join(".goose", "skills"), DetectMarkers: []string{filepath.Join(home, ".config", "goose")}, - DetectBinaries: nil, }, { ID: "opencode", @@ -107,15 +131,15 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".config", "opencode", "skills"), ProjectSkillsDir: filepath.Join(".agents", "skills"), DetectMarkers: []string{filepath.Join(home, ".config", "opencode")}, - DetectBinaries: nil, }, { - ID: "codex", - DisplayName: "Codex (OpenAI)", - GlobalSkillsDir: filepath.Join(home, ".codex", "skills"), - ProjectSkillsDir: filepath.Join(".agents", "skills"), - DetectMarkers: []string{os.Getenv("CODEX_HOME")}, - DetectBinaries: nil, + ID: "codex", + DisplayName: "Codex (OpenAI)", + GlobalSkillsDir: filepath.Join(home, ".codex", "skills"), + GlobalSkillsDirEnvVar: "CODEX_HOME", + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{"/etc/codex"}, + DetectMarkerEnvVars: []string{"CODEX_HOME"}, }, { ID: "windsurf", @@ -123,7 +147,6 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".windsurf", "skills"), ProjectSkillsDir: filepath.Join(".windsurf", "skills"), DetectMarkers: []string{filepath.Join(home, ".windsurf")}, - DetectBinaries: nil, }, { ID: "continue", @@ -131,7 +154,6 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".continue", "skills"), ProjectSkillsDir: filepath.Join(".continue", "skills"), DetectMarkers: []string{filepath.Join(home, ".continue")}, - DetectBinaries: nil, }, { ID: "amp", @@ -139,7 +161,6 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".config", "agents", "skills"), ProjectSkillsDir: filepath.Join(".agents", "skills"), DetectMarkers: []string{filepath.Join(home, ".config", "amp")}, - DetectBinaries: nil, }, { ID: "junie", @@ -147,7 +168,6 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".junie", "skills"), ProjectSkillsDir: filepath.Join(".junie", "skills"), DetectMarkers: []string{filepath.Join(home, ".junie")}, - DetectBinaries: nil, }, { ID: "kiro-cli", @@ -155,7 +175,6 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".kiro", "skills"), ProjectSkillsDir: filepath.Join(".kiro", "skills"), DetectMarkers: []string{filepath.Join(home, ".kiro")}, - DetectBinaries: nil, }, { ID: "cline", @@ -163,7 +182,6 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".agents", "skills"), ProjectSkillsDir: filepath.Join(".agents", "skills"), DetectMarkers: []string{filepath.Join(home, ".cline")}, - DetectBinaries: nil, }, { ID: "augment", @@ -171,7 +189,6 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".augment", "skills"), ProjectSkillsDir: filepath.Join(".augment", "skills"), DetectMarkers: []string{filepath.Join(home, ".augment")}, - DetectBinaries: nil, }, { ID: "aider-desk", @@ -179,7 +196,6 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".aider-desk", "skills"), ProjectSkillsDir: filepath.Join(".aider-desk", "skills"), DetectMarkers: []string{filepath.Join(home, ".aider-desk")}, - DetectBinaries: nil, }, { ID: "warp", @@ -187,31 +203,45 @@ func init() { GlobalSkillsDir: filepath.Join(home, ".config", "agents", "skills"), ProjectSkillsDir: filepath.Join(".agents", "skills"), DetectMarkers: []string{filepath.Join(home, ".warp")}, - DetectBinaries: nil, + }, + { + ID: "devin", + DisplayName: "Devin", + GlobalSkillsDir: filepath.Join(home, ".config", "devin", "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{filepath.Join(home, ".config", "devin")}, + }, + { + ID: "mistral-vibe", + DisplayName: "Mistral Vibe", + GlobalSkillsDir: filepath.Join(home, ".mistral-vibe", "skills"), + GlobalSkillsDirEnvVar: "VIBE_HOME", + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkerEnvVars: []string{"VIBE_HOME"}, }, { ID: "openhands", DisplayName: "OpenHands", GlobalSkillsDir: filepath.Join(home, ".openhands", "skills"), ProjectSkillsDir: filepath.Join(".openhands", "skills"), - DetectMarkers: nil, - DetectBinaries: nil, }, { ID: "trae", DisplayName: "Trae", GlobalSkillsDir: filepath.Join(home, ".trae", "skills"), ProjectSkillsDir: filepath.Join(".trae", "skills"), - DetectMarkers: nil, - DetectBinaries: nil, + }, + { + ID: "mux", + DisplayName: "Mux", + GlobalSkillsDir: filepath.Join(home, ".mux", "skills"), + ProjectSkillsDir: filepath.Join(".mux", "skills"), }, { ID: "universal", DisplayName: "Universal", GlobalSkillsDir: filepath.Join(home, ".agents", "skills"), ProjectSkillsDir: filepath.Join(".agents", "skills"), - DetectMarkers: nil, - DetectBinaries: nil, }, } } @@ -221,8 +251,6 @@ var ( detectedAgentsCache []AgentConfig ) -// DetectedAgents returns the subset of SupportedAgents that are installed on this machine. -// The universal agent is always included. Result is cached after the first call. func DetectedAgents() []AgentConfig { detectedAgentsOnce.Do(func() { for _, a := range SupportedAgents { @@ -234,9 +262,11 @@ func DetectedAgents() []AgentConfig { return detectedAgentsCache } -// FastPriorityAgents returns detected agents in the priority order used by --fast mode: -// claude-code, cursor, github-copilot, gemini-cli, then all other detected agents, -// with universal always appended last. +func ResetDetectedAgentsCache() { + detectedAgentsOnce = sync.Once{} + detectedAgentsCache = nil +} + func FastPriorityAgents() []AgentConfig { detected := DetectedAgents() diff --git a/internal/ai/skills/agent_test.go b/internal/ai/skills/agent_test.go index 32224e594..d18d79ece 100644 --- a/internal/ai/skills/agent_test.go +++ b/internal/ai/skills/agent_test.go @@ -74,6 +74,55 @@ func TestIsInstalled(t *testing.T) { } assert.True(t, a.IsInstalled()) }) + + t.Run("DetectMarkerEnvVars: returns true when env var points to existing path", func(t *testing.T) { + dir := t.TempDir() + t.Setenv("AUTH0_TEST_DETECT_HOME", dir) + a := AgentConfig{DetectMarkerEnvVars: []string{"AUTH0_TEST_DETECT_HOME"}} + assert.True(t, a.IsInstalled()) + }) + + t.Run("DetectMarkerEnvVars: returns false when env var is unset", func(t *testing.T) { + t.Setenv("AUTH0_TEST_DETECT_HOME_UNSET", "") + a := AgentConfig{DetectMarkerEnvVars: []string{"AUTH0_TEST_DETECT_HOME_UNSET"}} + assert.False(t, a.IsInstalled()) + }) + + t.Run("DetectMarkerEnvVars: returns false when env var points to non-existent path", func(t *testing.T) { + t.Setenv("AUTH0_TEST_DETECT_HOME", "/does/not/exist/for/sure/99999") + a := AgentConfig{DetectMarkerEnvVars: []string{"AUTH0_TEST_DETECT_HOME"}} + assert.False(t, a.IsInstalled()) + }) + + t.Run("DetectMarkerEnvVars: skips empty env var names", func(t *testing.T) { + a := AgentConfig{DetectMarkerEnvVars: []string{"", "ALSO_NOT_SET_SKIPS_99999"}} + assert.False(t, a.IsInstalled()) + }) +} + +func TestResolvedGlobalSkillsDir(t *testing.T) { + t.Run("returns GlobalSkillsDir when env var is unset", func(t *testing.T) { + t.Setenv("AUTH0_TEST_SKILLS_HOME", "") + a := AgentConfig{ + GlobalSkillsDir: "/fallback/skills", + GlobalSkillsDirEnvVar: "AUTH0_TEST_SKILLS_HOME", + } + assert.Equal(t, "/fallback/skills", a.ResolvedGlobalSkillsDir()) + }) + + t.Run("returns env var path when set", func(t *testing.T) { + t.Setenv("AUTH0_TEST_SKILLS_HOME", "/custom/home") + a := AgentConfig{ + GlobalSkillsDir: "/fallback/skills", + GlobalSkillsDirEnvVar: "AUTH0_TEST_SKILLS_HOME", + } + assert.Equal(t, filepath.Join("/custom/home", "skills"), a.ResolvedGlobalSkillsDir()) + }) + + t.Run("returns GlobalSkillsDir when GlobalSkillsDirEnvVar is empty", func(t *testing.T) { + a := AgentConfig{GlobalSkillsDir: "/fallback/skills"} + assert.Equal(t, "/fallback/skills", a.ResolvedGlobalSkillsDir()) + }) } func TestSupportedAgents(t *testing.T) { @@ -114,10 +163,25 @@ func TestSupportedAgents(t *testing.T) { assert.True(t, found) }) + t.Run("required agents are present", func(t *testing.T) { + required := []string{ + "claude-code", "cursor", "github-copilot", "gemini-cli", + "antigravity", "devin", "mistral-vibe", "mux", + "codex", "universal", + } + byID := make(map[string]bool, len(SupportedAgents)) + for _, a := range SupportedAgents { + byID[a.ID] = true + } + for _, id := range required { + assert.Truef(t, byID[id], "agent %s must be in SupportedAgents", id) + } + }) + t.Run("agents with no detection are detectable-never", func(t *testing.T) { - // Openhands, trae, and universal have nil markers/binaries meaning IsInstalled always - // returns false for them; they are included through explicit ID checks instead. - noDetectIDs := []string{"openhands", "trae", "universal"} + // openhands, trae, mux, and universal have nil markers/binaries meaning IsInstalled + // always returns false; they are included via explicit ID checks or --agent flag. + noDetectIDs := []string{"openhands", "trae", "mux", "universal"} byID := make(map[string]AgentConfig) for _, a := range SupportedAgents { byID[a.ID] = a @@ -127,8 +191,41 @@ func TestSupportedAgents(t *testing.T) { require.Truef(t, ok, "agent %s must be in SupportedAgents", id) assert.Nilf(t, a.DetectMarkers, "agent %s should have nil DetectMarkers", id) assert.Nilf(t, a.DetectBinaries, "agent %s should have nil DetectBinaries", id) + assert.Nilf(t, a.DetectMarkerEnvVars, "agent %s should have nil DetectMarkerEnvVars", id) + } + }) + + t.Run("codex uses CODEX_HOME env var for detection and skills dir", func(t *testing.T) { + byID := make(map[string]AgentConfig) + for _, a := range SupportedAgents { + byID[a.ID] = a + } + codex := byID["codex"] + assert.Equal(t, "CODEX_HOME", codex.GlobalSkillsDirEnvVar) + assert.Contains(t, codex.DetectMarkerEnvVars, "CODEX_HOME") + assert.Contains(t, codex.DetectMarkers, "/etc/codex") + }) + + t.Run("github-copilot does not use gh binary for detection", func(t *testing.T) { + byID := make(map[string]AgentConfig) + for _, a := range SupportedAgents { + byID[a.ID] = a + } + copilot := byID["github-copilot"] + for _, b := range copilot.DetectBinaries { + assert.NotEqual(t, "gh", b, "gh is the GitHub CLI, not Copilot; must not be used as a detection proxy") } }) + + t.Run("mistral-vibe uses VIBE_HOME env var", func(t *testing.T) { + byID := make(map[string]AgentConfig) + for _, a := range SupportedAgents { + byID[a.ID] = a + } + mv := byID["mistral-vibe"] + assert.Equal(t, "VIBE_HOME", mv.GlobalSkillsDirEnvVar) + assert.Contains(t, mv.DetectMarkerEnvVars, "VIBE_HOME") + }) } func TestDetectedAgents(t *testing.T) { @@ -161,6 +258,58 @@ func TestDetectedAgents(t *testing.T) { }) } +func TestResetDetectedAgentsCache(t *testing.T) { + t.Run("subsequent call after reset re-evaluates detection", func(t *testing.T) { + // Prime the cache. + first := DetectedAgents() + require.NotNil(t, first) + + // Reset should clear the cached result. + ResetDetectedAgentsCache() + + // A second call after reset should return a fresh (equal) result. + second := DetectedAgents() + assert.Equal(t, first, second) + }) + + t.Run("reset allows new filesystem state to be detected", func(t *testing.T) { + // Temporarily inject a fake agent that detects a temp dir. + dir := t.TempDir() + fake := AgentConfig{ + ID: "test-reset-agent", + DisplayName: "Test Reset Agent", + GlobalSkillsDir: filepath.Join(dir, "skills"), + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{filepath.Join(dir, "marker")}, + } + original := SupportedAgents + t.Cleanup(func() { + SupportedAgents = original + ResetDetectedAgentsCache() + }) + + // Without the marker, fake agent should not be detected. + ResetDetectedAgentsCache() + SupportedAgents = append(SupportedAgents, fake) + withoutMarker := DetectedAgents() + for _, a := range withoutMarker { + assert.NotEqual(t, "test-reset-agent", a.ID) + } + + // Create the marker and reset — fake agent should now be detected. + require.NoError(t, os.MkdirAll(filepath.Join(dir, "marker"), 0o755)) + ResetDetectedAgentsCache() + withMarker := DetectedAgents() + found := false + for _, a := range withMarker { + if a.ID == "test-reset-agent" { + found = true + } + } + assert.True(t, found, "agent should be detected after marker is created and cache is reset") + }) +} + func TestFastPriorityAgents(t *testing.T) { t.Run("universal is always last", func(t *testing.T) { result := FastPriorityAgents() From a42bd1c95d72661b773b763dd759f310d67295ee Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Tue, 2 Jun 2026 13:59:46 +0530 Subject: [PATCH 09/23] fix: added agents review fixes --- internal/ai/skills/agent.go | 58 ++++++++++++++++++++------------ internal/ai/skills/agent_test.go | 42 ++++++++++++++++++++--- 2 files changed, 75 insertions(+), 25 deletions(-) diff --git a/internal/ai/skills/agent.go b/internal/ai/skills/agent.go index c6da6ecff..8978f1ce6 100644 --- a/internal/ai/skills/agent.go +++ b/internal/ai/skills/agent.go @@ -1,6 +1,7 @@ package skills import ( + "errors" "os" "os/exec" "path/filepath" @@ -8,8 +9,8 @@ import ( ) type AgentConfig struct { - ID string - DisplayName string + ID string + DisplayName string GlobalSkillsDir string GlobalSkillsDirEnvVar string ProjectSkillsDir string @@ -18,13 +19,16 @@ type AgentConfig struct { DetectBinaries []string } -func (a AgentConfig) ResolvedGlobalSkillsDir() string { +func (a AgentConfig) ResolvedGlobalSkillsDir() (string, error) { if a.GlobalSkillsDirEnvVar != "" { if v := os.Getenv(a.GlobalSkillsDirEnvVar); v != "" { - return filepath.Join(v, "skills") + return filepath.Join(v, "skills"), nil } } - return a.GlobalSkillsDir + if a.GlobalSkillsDir == "" { + return "", errors.New("GlobalSkillsDirEnvVar must be set for: " + a.ID) + } + return a.GlobalSkillsDir, nil } func (a AgentConfig) IsInstalled() bool { @@ -94,7 +98,6 @@ func init() { filepath.Join(home, ".copilot"), filepath.Join(home, ".config", "github-copilot"), }, - DetectBinaries: []string{"code"}, }, { ID: "gemini-cli", @@ -133,13 +136,13 @@ func init() { DetectMarkers: []string{filepath.Join(home, ".config", "opencode")}, }, { - ID: "codex", - DisplayName: "Codex (OpenAI)", - GlobalSkillsDir: filepath.Join(home, ".codex", "skills"), + ID: "codex", + DisplayName: "Codex (OpenAI)", + GlobalSkillsDir: filepath.Join(home, ".codex", "skills"), GlobalSkillsDirEnvVar: "CODEX_HOME", - ProjectSkillsDir: filepath.Join(".agents", "skills"), - DetectMarkers: []string{"/etc/codex"}, - DetectMarkerEnvVars: []string{"CODEX_HOME"}, + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkers: []string{"/etc/codex"}, + DetectMarkerEnvVars: []string{"CODEX_HOME"}, }, { ID: "windsurf", @@ -212,12 +215,11 @@ func init() { DetectMarkers: []string{filepath.Join(home, ".config", "devin")}, }, { - ID: "mistral-vibe", - DisplayName: "Mistral Vibe", - GlobalSkillsDir: filepath.Join(home, ".mistral-vibe", "skills"), + ID: "mistral-vibe", + DisplayName: "Mistral Vibe", GlobalSkillsDirEnvVar: "VIBE_HOME", - ProjectSkillsDir: filepath.Join(".agents", "skills"), - DetectMarkerEnvVars: []string{"VIBE_HOME"}, + ProjectSkillsDir: filepath.Join(".agents", "skills"), + DetectMarkerEnvVars: []string{"VIBE_HOME"}, }, { ID: "openhands", @@ -247,23 +249,37 @@ func init() { } var ( - detectedAgentsOnce sync.Once + detectedAgentsMu sync.RWMutex + detectedAgentsDone bool detectedAgentsCache []AgentConfig ) func DetectedAgents() []AgentConfig { - detectedAgentsOnce.Do(func() { + detectedAgentsMu.RLock() + if detectedAgentsDone { + result := detectedAgentsCache + detectedAgentsMu.RUnlock() + return result + } + detectedAgentsMu.RUnlock() + + detectedAgentsMu.Lock() + defer detectedAgentsMu.Unlock() + if !detectedAgentsDone { for _, a := range SupportedAgents { if a.ID == "universal" || a.IsInstalled() { detectedAgentsCache = append(detectedAgentsCache, a) } } - }) + detectedAgentsDone = true + } return detectedAgentsCache } func ResetDetectedAgentsCache() { - detectedAgentsOnce = sync.Once{} + detectedAgentsMu.Lock() + defer detectedAgentsMu.Unlock() + detectedAgentsDone = false detectedAgentsCache = nil } diff --git a/internal/ai/skills/agent_test.go b/internal/ai/skills/agent_test.go index d18d79ece..7561eaf4b 100644 --- a/internal/ai/skills/agent_test.go +++ b/internal/ai/skills/agent_test.go @@ -107,7 +107,9 @@ func TestResolvedGlobalSkillsDir(t *testing.T) { GlobalSkillsDir: "/fallback/skills", GlobalSkillsDirEnvVar: "AUTH0_TEST_SKILLS_HOME", } - assert.Equal(t, "/fallback/skills", a.ResolvedGlobalSkillsDir()) + got, err := a.ResolvedGlobalSkillsDir() + assert.NoError(t, err) + assert.Equal(t, "/fallback/skills", got) }) t.Run("returns env var path when set", func(t *testing.T) { @@ -116,12 +118,43 @@ func TestResolvedGlobalSkillsDir(t *testing.T) { GlobalSkillsDir: "/fallback/skills", GlobalSkillsDirEnvVar: "AUTH0_TEST_SKILLS_HOME", } - assert.Equal(t, filepath.Join("/custom/home", "skills"), a.ResolvedGlobalSkillsDir()) + got, err := a.ResolvedGlobalSkillsDir() + assert.NoError(t, err) + assert.Equal(t, filepath.Join("/custom/home", "skills"), got) }) t.Run("returns GlobalSkillsDir when GlobalSkillsDirEnvVar is empty", func(t *testing.T) { a := AgentConfig{GlobalSkillsDir: "/fallback/skills"} - assert.Equal(t, "/fallback/skills", a.ResolvedGlobalSkillsDir()) + got, err := a.ResolvedGlobalSkillsDir() + assert.NoError(t, err) + assert.Equal(t, "/fallback/skills", got) + }) + + t.Run("returns error when GlobalSkillsDir is empty and env var unset", func(t *testing.T) { + a := AgentConfig{ID: "test-agent"} + _, err := a.ResolvedGlobalSkillsDir() + assert.EqualError(t, err, "GlobalSkillsDirEnvVar must be set for: test-agent") + }) + + t.Run("returns env var path when GlobalSkillsDir is empty but env var is set", func(t *testing.T) { + t.Setenv("AUTH0_TEST_SKILLS_HOME", "/custom/home") + a := AgentConfig{ + ID: "test-agent", + GlobalSkillsDirEnvVar: "AUTH0_TEST_SKILLS_HOME", + } + got, err := a.ResolvedGlobalSkillsDir() + assert.NoError(t, err) + assert.Equal(t, filepath.Join("/custom/home", "skills"), got) + }) + + t.Run("mistral-vibe returns error when VIBE_HOME is not set", func(t *testing.T) { + t.Setenv("VIBE_HOME", "") + a := AgentConfig{ + ID: "mistral-vibe", + GlobalSkillsDirEnvVar: "VIBE_HOME", + } + _, err := a.ResolvedGlobalSkillsDir() + assert.EqualError(t, err, "GlobalSkillsDirEnvVar must be set for: mistral-vibe") }) } @@ -139,7 +172,8 @@ func TestSupportedAgents(t *testing.T) { t.Run("all agents have non-empty skill dirs", func(t *testing.T) { for _, a := range SupportedAgents { - assert.NotEmptyf(t, a.GlobalSkillsDir, "agent %s GlobalSkillsDir must not be empty", a.ID) + hasGlobalDir := a.GlobalSkillsDir != "" || a.GlobalSkillsDirEnvVar != "" + assert.Truef(t, hasGlobalDir, "agent %s must have GlobalSkillsDir or GlobalSkillsDirEnvVar", a.ID) assert.NotEmptyf(t, a.ProjectSkillsDir, "agent %s ProjectSkillsDir must not be empty", a.ID) } }) From 18cf9767db79829adc2617ee4aa4a2a514046a17 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Tue, 2 Jun 2026 18:00:35 +0530 Subject: [PATCH 10/23] fix: download internal review fixes --- internal/ai/skills/download.go | 146 +++++++------- internal/ai/skills/download_test.go | 291 +++++++++++++++++++++++++--- 2 files changed, 343 insertions(+), 94 deletions(-) diff --git a/internal/ai/skills/download.go b/internal/ai/skills/download.go index fae60ea69..d31ed1677 100644 --- a/internal/ai/skills/download.go +++ b/internal/ai/skills/download.go @@ -4,6 +4,7 @@ import ( "archive/tar" "archive/zip" "compress/gzip" + "context" "encoding/json" "fmt" "io" @@ -11,6 +12,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "time" ) @@ -20,9 +22,15 @@ const ( agentSkillsAPI = "https://api.github.com/repos/auth0/agent-skills/commits/" pluginSubtreePath = "plugins/auth0" skillsHTTPTimeout = 60 * time.Second - maxSkillsDownload = 100 * 1024 * 1024 // 100 MB. + gitCmdTimeout = 120 * time.Second + minGitMajor = 2 + minGitMinor = 25 ) +// maxSkillsDownload is the per-archive byte limit for HTTP downloads. Declared as a var so +// tests can override it without allocating a 100 MB body. +var maxSkillsDownload int64 = 100 * 1024 * 1024 // 100 MB. + var skillsHTTPClient = &http.Client{Timeout: skillsHTTPTimeout} // DownloadPlugin downloads the auth0 agent-skills plugin into targetDir using the best @@ -32,19 +40,62 @@ func DownloadPlugin(targetDir, ref string) (string, error) { ref = "main" } - if _, err := exec.LookPath("git"); err == nil { - sha, err := downloadViaGit(targetDir, ref) - if err == nil { - return sha, nil + if err := os.MkdirAll(targetDir, 0o755); err != nil { + return "", fmt.Errorf("create target dir: %w", err) + } + + if _, err := exec.LookPath("git"); err == nil && checkGitVersion() == nil { + if sha, err := downloadViaGit(targetDir, ref); err == nil { + return sha, checkNonEmpty(targetDir) } } - sha, err := downloadViaTarGz(targetDir, ref) - if err == nil { - return sha, nil + if sha, err := downloadViaTarGz(targetDir, ref); err == nil { + return sha, checkNonEmpty(targetDir) } - return downloadViaZip(targetDir, ref) + sha, err := downloadViaZip(targetDir, ref) + if err != nil { + return "", err + } + return sha, checkNonEmpty(targetDir) +} + +// checkNonEmpty returns an error if dir contains no entries. +func checkNonEmpty(dir string) error { + entries, err := os.ReadDir(dir) + if err != nil { + return fmt.Errorf("check extraction result: %w", err) + } + if len(entries) == 0 { + return fmt.Errorf("extraction produced no files in %s (archive prefix may not match)", dir) + } + return nil +} + +// checkGitVersion returns an error if git is not found or is older than 2.25. +func checkGitVersion() error { + out, err := exec.Command("git", "--version").Output() + if err != nil { + return fmt.Errorf("git --version: %w", err) + } + fields := strings.Fields(strings.TrimSpace(string(out))) + if len(fields) < 3 { + return fmt.Errorf("unexpected git --version output: %s", strings.TrimSpace(string(out))) + } + vParts := strings.SplitN(fields[2], ".", 3) + if len(vParts) < 2 { + return fmt.Errorf("cannot parse git version: %s", fields[2]) + } + major, err1 := strconv.Atoi(vParts[0]) + minor, err2 := strconv.Atoi(vParts[1]) + if err1 != nil || err2 != nil { + return fmt.Errorf("cannot parse git version: %s", fields[2]) + } + if major < minGitMajor || (major == minGitMajor && minor < minGitMinor) { + return fmt.Errorf("git >= %d.%d required, found %s", minGitMajor, minGitMinor, fields[2]) + } + return nil } // fetchCommitSHA fetches the latest commit SHA for ref from the GitHub API. @@ -54,6 +105,9 @@ func fetchCommitSHA(ref string) (string, error) { return "", err } req.Header.Set("Accept", "application/vnd.github.v3+json") + if token := os.Getenv("GITHUB_TOKEN"); token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } resp, err := skillsHTTPClient.Do(req) if err != nil { @@ -77,25 +131,24 @@ func fetchCommitSHA(ref string) (string, error) { return payload.SHA, nil } -// downloadViaGit uses git sparse-checkout to download only plugins/auth0. +// downloadViaGit uses git sparse-checkout to download only plugins/auth0 directly into targetDir. func downloadViaGit(targetDir, ref string) (string, error) { - tmpDir, err := os.MkdirTemp("", "auth0-agent-skills-*") - if err != nil { - return "", fmt.Errorf("failed to create temp dir: %w", err) - } - defer os.RemoveAll(tmpDir) - run := func(args ...string) (string, error) { - cmd := exec.Command("git", args...) - cmd.Dir = tmpDir + ctx, cancel := context.WithTimeout(context.Background(), gitCmdTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = targetDir out, err := cmd.CombinedOutput() if err != nil { + if ctx.Err() != nil { + return "", fmt.Errorf("git %s: timed out after %s", strings.Join(args, " "), gitCmdTimeout) + } return "", fmt.Errorf("git %s: %w\n%s", strings.Join(args, " "), err, out) } return strings.TrimSpace(string(out)), nil } - if _, err := run("clone", "--no-checkout", "--depth", "1", "--filter=blob:none", + if _, err := run("clone", "--no-checkout", "--depth", "1", "--filter=blob:none", "--branch", ref, agentSkillsRepo+".git", "."); err != nil { return "", err } @@ -108,17 +161,7 @@ func downloadViaGit(targetDir, ref string) (string, error) { return "", err } - sha, err := run("rev-parse", "HEAD") - if err != nil { - return "", err - } - - srcDir := filepath.Join(tmpDir, pluginSubtreePath) - if err := mergeDir(srcDir, targetDir); err != nil { - return "", err - } - - return sha, nil + return run("rev-parse", "HEAD") } // fetchToTempFile downloads url into a new temp file and returns it open and seeked to the @@ -147,6 +190,12 @@ func fetchToTempFile(url, pattern, label string) (*os.File, int64, error) { return nil, 0, fmt.Errorf("failed to save %s: %w", label, err) } + if size == maxSkillsDownload { + _ = f.Close() + _ = os.Remove(f.Name()) + return nil, 0, fmt.Errorf("%s: archive exceeds size limit of %d bytes", label, maxSkillsDownload) + } + if _, err := f.Seek(0, io.SeekStart); err != nil { _ = f.Close() _ = os.Remove(f.Name()) @@ -279,40 +328,3 @@ func extractZipSubtree(zipPath string, size int64, prefix, destDir string) error return nil } -// mergeDir copies all files from src into dst, creating directories as needed. -func mergeDir(src, dst string) error { - return filepath.Walk(src, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - - rel, err := filepath.Rel(src, path) - if err != nil { - return err - } - - target := filepath.Join(dst, rel) - - if info.IsDir() { - return os.MkdirAll(target, 0o755) - } - - if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { - return err - } - - in, err := os.Open(path) - if err != nil { - return err - } - out, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, info.Mode()) - if err != nil { - _ = in.Close() - return err - } - _, copyErr := io.Copy(out, in) - _ = in.Close() - _ = out.Close() - return copyErr - }) -} diff --git a/internal/ai/skills/download_test.go b/internal/ai/skills/download_test.go index 34a1f2e8f..e352ee037 100644 --- a/internal/ai/skills/download_test.go +++ b/internal/ai/skills/download_test.go @@ -7,9 +7,11 @@ import ( "compress/gzip" "encoding/json" "errors" + "fmt" "io" "net/http" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -203,33 +205,6 @@ func TestExtractZipSubtree(t *testing.T) { }) } -// --- mergeDir ---. - -func TestMergeDir(t *testing.T) { - t.Run("copies flat files", func(t *testing.T) { - src, dst := t.TempDir(), t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(src, "a.txt"), []byte("aaa"), 0o644)) - require.NoError(t, mergeDir(src, dst)) - assertFileContent(t, filepath.Join(dst, "a.txt"), "aaa") - }) - - t.Run("copies nested files and creates subdirectories", func(t *testing.T) { - src, dst := t.TempDir(), t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(src, "sub", "deep"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(src, "sub", "deep", "b.txt"), []byte("bbb"), 0o644)) - require.NoError(t, mergeDir(src, dst)) - assertFileContent(t, filepath.Join(dst, "sub", "deep", "b.txt"), "bbb") - }) - - t.Run("overwrites existing destination files", func(t *testing.T) { - src, dst := t.TempDir(), t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(src, "f.txt"), []byte("new"), 0o644)) - require.NoError(t, os.WriteFile(filepath.Join(dst, "f.txt"), []byte("old"), 0o644)) - require.NoError(t, mergeDir(src, dst)) - assertFileContent(t, filepath.Join(dst, "f.txt"), "new") - }) -} - // --- fetchToTempFile ---. func TestFetchToTempFile(t *testing.T) { @@ -314,4 +289,266 @@ func TestFetchCommitSHA(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "github API request failed") }) + + t.Run("sends Authorization header when GITHUB_TOKEN is set", func(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "test-token-xyz") + var capturedAuth string + setHTTPClient(t, func(r *http.Request) (*http.Response, error) { + capturedAuth = r.Header.Get("Authorization") + body, _ := json.Marshal(map[string]string{"sha": "abc123"}) + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(body))}, nil + }) + _, err := fetchCommitSHA("main") + require.NoError(t, err) + assert.Equal(t, "Bearer test-token-xyz", capturedAuth) + }) + + t.Run("omits Authorization header when GITHUB_TOKEN is not set", func(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "") + var capturedAuth string + setHTTPClient(t, func(r *http.Request) (*http.Response, error) { + capturedAuth = r.Header.Get("Authorization") + body, _ := json.Marshal(map[string]string{"sha": "abc123"}) + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(body))}, nil + }) + _, err := fetchCommitSHA("main") + require.NoError(t, err) + assert.Empty(t, capturedAuth) + }) +} + +// --- fetchToTempFile truncation ---. + +func TestFetchToTempFile_Truncation(t *testing.T) { + t.Run("returns error when response body hits size limit", func(t *testing.T) { + orig := maxSkillsDownload + maxSkillsDownload = 10 + t.Cleanup(func() { maxSkillsDownload = orig }) + + // Body has more bytes than the limit; LimitReader will stop at exactly 10 bytes. + body := strings.Repeat("x", 20) + setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body))}, nil + }) + _, _, err := fetchToTempFile("http://example.com/f", "test-*", "test") + require.Error(t, err) + assert.Contains(t, err.Error(), "exceeds size limit") + }) + + t.Run("succeeds when response body is exactly one byte under limit", func(t *testing.T) { + orig := maxSkillsDownload + maxSkillsDownload = 10 + t.Cleanup(func() { maxSkillsDownload = orig }) + + body := strings.Repeat("x", 9) // 9 < 10 + setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body))}, nil + }) + f, size, err := fetchToTempFile("http://example.com/f", "test-*", "test") + require.NoError(t, err) + t.Cleanup(func() { f.Close(); os.Remove(f.Name()) }) + assert.Equal(t, int64(9), size) + }) +} + +// --- checkNonEmpty ---. + +func TestCheckNonEmpty(t *testing.T) { + t.Run("returns error for empty directory", func(t *testing.T) { + dir := t.TempDir() + err := checkNonEmpty(dir) + require.Error(t, err) + assert.Contains(t, err.Error(), "no files") + }) + + t.Run("returns nil for directory with at least one entry", func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "file.txt"), []byte("x"), 0o644)) + assert.NoError(t, checkNonEmpty(dir)) + }) + + t.Run("returns error for non-existent directory", func(t *testing.T) { + err := checkNonEmpty(filepath.Join(t.TempDir(), "does-not-exist")) + require.Error(t, err) + }) +} + +// --- downloadViaTarGz ---. + +func TestDownloadViaTarGz(t *testing.T) { + const ref = "main" + const wantSHA = "deadbeefcafe" + prefix := fmt.Sprintf("auth0-agent-skills-%s/%s/", ref, pluginSubtreePath) + + makeMockTransport := func(tarData []byte, sha string) roundTripFunc { + return func(r *http.Request) (*http.Response, error) { + if r.URL.Host == "codeload.github.com" { + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(tarData))}, nil + } + body, _ := json.Marshal(map[string]string{"sha": sha}) + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(body))}, nil + } + } + + t.Run("extracts subtree and returns commit SHA", func(t *testing.T) { + tarData := makeTarGz(t, map[string]string{ + prefix + "skill-a/SKILL.md": "# skill-a", + prefix + "skill-b/SKILL.md": "# skill-b", + }) + setHTTPClient(t, makeMockTransport(tarData, wantSHA)) + + dest := t.TempDir() + gotSHA, err := downloadViaTarGz(dest, ref) + require.NoError(t, err) + assert.Equal(t, wantSHA, gotSHA) + assertFileContent(t, filepath.Join(dest, "skill-a", "SKILL.md"), "# skill-a") + assertFileContent(t, filepath.Join(dest, "skill-b", "SKILL.md"), "# skill-b") + }) + + t.Run("returns error when SHA API call fails", func(t *testing.T) { + tarData := makeTarGz(t, map[string]string{prefix + "skill-a/SKILL.md": "x"}) + setHTTPClient(t, func(r *http.Request) (*http.Response, error) { + if r.URL.Host == "codeload.github.com" { + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(tarData))}, nil + } + return &http.Response{StatusCode: http.StatusForbidden, Body: io.NopCloser(strings.NewReader(""))}, nil + }) + _, err := downloadViaTarGz(t.TempDir(), ref) + require.Error(t, err) + assert.Contains(t, err.Error(), "403") + }) + + t.Run("returns error when download fails", func(t *testing.T) { + setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusInternalServerError, Body: io.NopCloser(strings.NewReader(""))}, nil + }) + _, err := downloadViaTarGz(t.TempDir(), ref) + require.Error(t, err) + }) +} + +// --- downloadViaZip ---. + +func TestDownloadViaZip(t *testing.T) { + const ref = "main" + const wantSHA = "cafebabe1234" + prefix := fmt.Sprintf("auth0-agent-skills-%s/%s/", ref, pluginSubtreePath) + + makeMockTransport := func(zipPath string, sha string) roundTripFunc { + zipData, err := os.ReadFile(zipPath) + if err != nil { + panic(err) + } + return func(r *http.Request) (*http.Response, error) { + if r.URL.Host == "github.com" { + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(zipData))}, nil + } + body, _ := json.Marshal(map[string]string{"sha": sha}) + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(body))}, nil + } + } + + t.Run("extracts subtree and returns commit SHA", func(t *testing.T) { + zipPath, _ := makeZip(t, map[string]string{ + prefix + "skill-x/SKILL.md": "# skill-x", + }) + setHTTPClient(t, makeMockTransport(zipPath, wantSHA)) + + dest := t.TempDir() + gotSHA, err := downloadViaZip(dest, ref) + require.NoError(t, err) + assert.Equal(t, wantSHA, gotSHA) + assertFileContent(t, filepath.Join(dest, "skill-x", "SKILL.md"), "# skill-x") + }) + + t.Run("returns error when download fails", func(t *testing.T) { + setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { + return &http.Response{StatusCode: http.StatusNotFound, Body: io.NopCloser(strings.NewReader(""))}, nil + }) + _, err := downloadViaZip(t.TempDir(), ref) + require.Error(t, err) + }) +} + +// --- DownloadPlugin ---. + +func TestDownloadPlugin_EmptyExtraction(t *testing.T) { + // When the archive contains no entries matching the expected prefix, checkNonEmpty + // should cause the strategy to fail, and DownloadPlugin returns an error. + // Skip when git is usable: DownloadPlugin will clone from the real repo and succeed, + // bypassing the HTTP mock strategies entirely. + if _, err := exec.LookPath("git"); err == nil && checkGitVersion() == nil { + t.Skip("git is available; DownloadPlugin uses git strategy which bypasses HTTP mock") + } + + const ref = "main" + + // Tar.gz with wrong prefix (no matching subtree). + wrongPrefixTar := makeTarGz(t, map[string]string{ + "completely-wrong-prefix/file.txt": "content", + }) + // ZIP with wrong prefix. + wrongPrefixZip, _ := makeZip(t, map[string]string{ + "completely-wrong-prefix/file.txt": "content", + }) + wrongPrefixZipData, err := os.ReadFile(wrongPrefixZip) + require.NoError(t, err) + + setHTTPClient(t, func(r *http.Request) (*http.Response, error) { + switch r.URL.Host { + case "codeload.github.com": + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(wrongPrefixTar))}, nil + case "github.com": + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(wrongPrefixZipData))}, nil + default: + // SHA API — shouldn't be reached if extraction fails. + body, _ := json.Marshal(map[string]string{"sha": "abc"}) + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(body))}, nil + } + }) + + base := t.TempDir() + targetDir := filepath.Join(base, "auth0") + _, err = DownloadPlugin(targetDir, ref) + // All three strategies should fail (git is skipped if not in PATH or fails network; + // tar.gz and ZIP produce empty dirs which checkNonEmpty rejects). + require.Error(t, err) +} + +func TestDownloadPlugin_CreatesMissingTargetDir(t *testing.T) { + const ref = "main" + const wantSHA = "abc123" + prefix := fmt.Sprintf("auth0-agent-skills-%s/%s/", ref, pluginSubtreePath) + tarData := makeTarGz(t, map[string]string{ + prefix + "skill-a/SKILL.md": "# skill-a", + }) + zipPath, _ := makeZip(t, map[string]string{ + prefix + "skill-a/SKILL.md": "# skill-a", + }) + zipData, err := os.ReadFile(zipPath) + require.NoError(t, err) + + setHTTPClient(t, func(r *http.Request) (*http.Response, error) { + switch r.URL.Host { + case "codeload.github.com": + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(tarData))}, nil + case "github.com": + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(zipData))}, nil + default: + body, _ := json.Marshal(map[string]string{"sha": wantSHA}) + return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(body))}, nil + } + }) + + // targetDir is deeply nested and does not exist yet. + targetDir := filepath.Join(t.TempDir(), "deep", "nested", "auth0") + _, err = DownloadPlugin(targetDir, ref) + // If git is available and succeeds via real network, sha may differ; we just check + // that the call succeeded and targetDir was created with at least one entry. + if err == nil { + entries, readErr := os.ReadDir(targetDir) + require.NoError(t, readErr) + assert.NotEmpty(t, entries, "targetDir must contain extracted files") + } + // If git fails in test environment (no network), tar.gz or ZIP mock should succeed. } From e2d3497764841b6e96405c6008a99a44e3d6588a Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Wed, 3 Jun 2026 14:20:45 +0530 Subject: [PATCH 11/23] fix: targetDir clone and ref tags vs branches prefix --- internal/ai/skills/download.go | 156 ++++++++++++++++++++++++++++----- 1 file changed, 135 insertions(+), 21 deletions(-) diff --git a/internal/ai/skills/download.go b/internal/ai/skills/download.go index d31ed1677..b2d375751 100644 --- a/internal/ai/skills/download.go +++ b/internal/ai/skills/download.go @@ -35,30 +35,57 @@ var skillsHTTPClient = &http.Client{Timeout: skillsHTTPTimeout} // DownloadPlugin downloads the auth0 agent-skills plugin into targetDir using the best // available strategy: git sparse-checkout > tar.gz > ZIP. Returns the commit SHA. +// All intermediate work happens in a system temp directory; targetDir is only written +// once everything succeeds. func DownloadPlugin(targetDir, ref string) (string, error) { if ref == "" { ref = "main" } - if err := os.MkdirAll(targetDir, 0o755); err != nil { - return "", fmt.Errorf("create target dir: %w", err) + tmpDir, err := os.MkdirTemp("", "auth0-skills-*") + if err != nil { + return "", fmt.Errorf("create temp dir: %w", err) } + defer os.RemoveAll(tmpDir) // no-op if renamed to targetDir below - if _, err := exec.LookPath("git"); err == nil && checkGitVersion() == nil { - if sha, err := downloadViaGit(targetDir, ref); err == nil { - return sha, checkNonEmpty(targetDir) + sha, dlErr := func() (string, error) { + if _, err := exec.LookPath("git"); err == nil && checkGitVersion() == nil { + if sha, err := downloadViaGit(tmpDir, ref); err == nil { + return sha, nil + } } + if sha, err := downloadViaTarGz(tmpDir, ref); err == nil { + return sha, nil + } + return downloadViaZip(tmpDir, ref) + }() + if dlErr != nil { + return "", dlErr } - if sha, err := downloadViaTarGz(targetDir, ref); err == nil { - return sha, checkNonEmpty(targetDir) + if err := checkNonEmpty(tmpDir); err != nil { + return "", err } - sha, err := downloadViaZip(targetDir, ref) - if err != nil { - return "", err + if err := os.MkdirAll(filepath.Dir(targetDir), 0o755); err != nil { + return "", fmt.Errorf("create parent dir: %w", err) + } + + os.RemoveAll(targetDir) + + // Attempt atomic rename (succeeds when /tmp and targetDir share a filesystem). + if err := os.Rename(tmpDir, targetDir); err == nil { + return sha, nil + } + + // Cross-filesystem fallback: copy content into a freshly created targetDir. + if err := os.MkdirAll(targetDir, 0o755); err != nil { + return "", fmt.Errorf("create target dir: %w", err) } - return sha, checkNonEmpty(targetDir) + if err := mergeDir(tmpDir, targetDir); err != nil { + return "", fmt.Errorf("install to target dir: %w", err) + } + return sha, nil } // checkNonEmpty returns an error if dir contains no entries. @@ -75,7 +102,9 @@ func checkNonEmpty(dir string) error { // checkGitVersion returns an error if git is not found or is older than 2.25. func checkGitVersion() error { - out, err := exec.Command("git", "--version").Output() + ctx, cancel := context.WithTimeout(context.Background(), gitCmdTimeout) + defer cancel() + out, err := exec.CommandContext(ctx, "git", "--version").Output() if err != nil { return fmt.Errorf("git --version: %w", err) } @@ -131,13 +160,20 @@ func fetchCommitSHA(ref string) (string, error) { return payload.SHA, nil } -// downloadViaGit uses git sparse-checkout to download only plugins/auth0 directly into targetDir. +// downloadViaGit clones into a temp directory, then promotes the contents of +// plugins/auth0/ into targetDir so the layout matches the tar.gz/ZIP strategies. func downloadViaGit(targetDir, ref string) (string, error) { + cloneDir, err := os.MkdirTemp("", "auth0-agent-skills-git-*") + if err != nil { + return "", fmt.Errorf("create git clone dir: %w", err) + } + defer os.RemoveAll(cloneDir) + run := func(args ...string) (string, error) { ctx, cancel := context.WithTimeout(context.Background(), gitCmdTimeout) defer cancel() cmd := exec.CommandContext(ctx, "git", args...) - cmd.Dir = targetDir + cmd.Dir = cloneDir out, err := cmd.CombinedOutput() if err != nil { if ctx.Err() != nil { @@ -161,7 +197,24 @@ func downloadViaGit(targetDir, ref string) (string, error) { return "", err } - return run("rev-parse", "HEAD") + sha, err := run("rev-parse", "HEAD") + if err != nil { + return "", err + } + + // Promote plugins/auth0 into targetDir by rename within /tmp — no data copy. + // targetDir was created empty by the caller; remove it so the rename can take its path. + subtreeSrc := filepath.Join(cloneDir, filepath.FromSlash(pluginSubtreePath)) + if err := os.Remove(targetDir); err != nil { + return "", fmt.Errorf("clear temp dir for promotion: %w", err) + } + if err := os.Rename(subtreeSrc, targetDir); err != nil { + // Restore the empty dir so fallback strategies in the caller can still use this path. + _ = os.MkdirAll(targetDir, 0o755) + return "", fmt.Errorf("promote git subtree: %w", err) + } + + return sha, nil } // fetchToTempFile downloads url into a new temp file and returns it open and seeked to the @@ -205,9 +258,14 @@ func fetchToTempFile(url, pattern, label string) (*os.File, int64, error) { return f, size, nil } -// downloadViaTarGz downloads the archive from codeload.github.com and extracts the subtree. +// downloadViaTarGz fetches the commit SHA first, then downloads and extracts the tar.gz archive. func downloadViaTarGz(targetDir, ref string) (string, error) { - url := fmt.Sprintf("https://codeload.github.com/auth0/agent-skills/tar.gz/refs/heads/%s", ref) + sha, err := fetchCommitSHA(ref) + if err != nil { + return "", err + } + + url := fmt.Sprintf("https://codeload.github.com/auth0/agent-skills/tar.gz/%s", ref) f, _, err := fetchToTempFile(url, "auth0-agent-skills-*.tar.gz", "tar.gz") if err != nil { return "", err @@ -220,12 +278,17 @@ func downloadViaTarGz(targetDir, ref string) (string, error) { return "", err } - return fetchCommitSHA(ref) + return sha, nil } -// downloadViaZip downloads the ZIP archive from github.com and extracts the subtree. +// downloadViaZip fetches the commit SHA first, then downloads and extracts the ZIP archive. func downloadViaZip(targetDir, ref string) (string, error) { - url := fmt.Sprintf("%s/archive/refs/heads/%s.zip", agentSkillsRepo, ref) + sha, err := fetchCommitSHA(ref) + if err != nil { + return "", err + } + + url := fmt.Sprintf("%s/archive/%s.zip", agentSkillsRepo, ref) f, size, err := fetchToTempFile(url, "auth0-agent-skills-*.zip", "ZIP") if err != nil { return "", err @@ -238,7 +301,58 @@ func downloadViaZip(targetDir, ref string) (string, error) { return "", err } - return fetchCommitSHA(ref) + return sha, nil +} + +// mergeDir recursively copies the contents of src into dst. +func mergeDir(src, dst string) error { + entries, err := os.ReadDir(src) + if err != nil { + return err + } + for _, entry := range entries { + srcPath := filepath.Join(src, entry.Name()) + dstPath := filepath.Join(dst, entry.Name()) + if entry.IsDir() { + if err := os.MkdirAll(dstPath, 0o755); err != nil { + return err + } + if err := mergeDir(srcPath, dstPath); err != nil { + return err + } + } else { + info, err := entry.Info() + if err != nil { + return err + } + if err := copyFile(srcPath, dstPath, info.Mode()); err != nil { + return err + } + } + } + return nil +} + +// copyFile copies src to dst with the given permission mode. +func copyFile(src, dst string, mode os.FileMode) error { + if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + return err + } + in, err := os.Open(src) + if err != nil { + return err + } + defer in.Close() + out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, mode) + if err != nil { + return err + } + _, copyErr := io.Copy(out, in) + closeErr := out.Close() + if copyErr != nil { + return copyErr + } + return closeErr } // ExtractEntry writes a single archive entry to destDir. IsDir and mode describe the entry; From 0135e58011386d88f6343c3bd3dbbe670d7fcb22 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Thu, 4 Jun 2026 09:01:27 +0530 Subject: [PATCH 12/23] fix: download module, error handling, symlink corruption in mergeDir, special char handling in ref URL --- internal/ai/skills/download.go | 68 ++++++++---- internal/ai/skills/download_test.go | 157 +++++++++++++++++++++++----- 2 files changed, 176 insertions(+), 49 deletions(-) diff --git a/internal/ai/skills/download.go b/internal/ai/skills/download.go index b2d375751..2c88c139e 100644 --- a/internal/ai/skills/download.go +++ b/internal/ai/skills/download.go @@ -31,6 +31,13 @@ const ( // tests can override it without allocating a 100 MB body. var maxSkillsDownload int64 = 100 * 1024 * 1024 // 100 MB. +// agentSkillsGitURL is the URL used by downloadViaGit. Declared as a var so tests can +// point it at a local bare repository instead of the real GitHub remote. +var agentSkillsGitURL = agentSkillsRepo + ".git" + +// gitLookPath is exec.LookPath by default; tests override it to force HTTP fallback strategies. +var gitLookPath = exec.LookPath + var skillsHTTPClient = &http.Client{Timeout: skillsHTTPTimeout} // DownloadPlugin downloads the auth0 agent-skills plugin into targetDir using the best @@ -49,21 +56,31 @@ func DownloadPlugin(targetDir, ref string) (string, error) { defer os.RemoveAll(tmpDir) // no-op if renamed to targetDir below sha, dlErr := func() (string, error) { - if _, err := exec.LookPath("git"); err == nil && checkGitVersion() == nil { + var errs []string + if _, err := gitLookPath("git"); err == nil && checkGitVersion() == nil { if sha, err := downloadViaGit(tmpDir, ref); err == nil { return sha, nil + } else { + errs = append(errs, "git: "+err.Error()) } } if sha, err := downloadViaTarGz(tmpDir, ref); err == nil { return sha, nil + } else { + errs = append(errs, "tar.gz: "+err.Error()) } - return downloadViaZip(tmpDir, ref) + if sha, err := downloadViaZip(tmpDir, ref); err == nil { + return sha, nil + } else { + errs = append(errs, "zip: "+err.Error()) + } + return "", fmt.Errorf("all download strategies failed: %s", strings.Join(errs, "; ")) }() if dlErr != nil { return "", dlErr } - if err := checkNonEmpty(tmpDir); err != nil { + if err := checkHasSkills(tmpDir); err != nil { return "", err } @@ -88,14 +105,11 @@ func DownloadPlugin(targetDir, ref string) (string, error) { return sha, nil } -// checkNonEmpty returns an error if dir contains no entries. -func checkNonEmpty(dir string) error { - entries, err := os.ReadDir(dir) - if err != nil { - return fmt.Errorf("check extraction result: %w", err) - } - if len(entries) == 0 { - return fmt.Errorf("extraction produced no files in %s (archive prefix may not match)", dir) +// checkHasSkills returns an error if dir/skills/ does not exist or contains no entries. +func checkHasSkills(dir string) error { + entries, err := os.ReadDir(filepath.Join(dir, "skills")) + if err != nil || len(entries) == 0 { + return fmt.Errorf("no skills found under %s/skills/ (archive prefix may not match)", dir) } return nil } @@ -185,7 +199,7 @@ func downloadViaGit(targetDir, ref string) (string, error) { } if _, err := run("clone", "--no-checkout", "--depth", "1", "--filter=blob:none", "--branch", ref, - agentSkillsRepo+".git", "."); err != nil { + agentSkillsGitURL, "."); err != nil { return "", err } @@ -202,10 +216,10 @@ func downloadViaGit(targetDir, ref string) (string, error) { return "", err } - // Promote plugins/auth0 into targetDir by rename within /tmp — no data copy. - // targetDir was created empty by the caller; remove it so the rename can take its path. + // Promote plugins/auth0/ into targetDir (the DownloadPlugin temp directory) by rename. + // Remove targetDir first so the rename can take its place. subtreeSrc := filepath.Join(cloneDir, filepath.FromSlash(pluginSubtreePath)) - if err := os.Remove(targetDir); err != nil { + if err := os.RemoveAll(targetDir); err != nil { return "", fmt.Errorf("clear temp dir for promotion: %w", err) } if err := os.Rename(subtreeSrc, targetDir); err != nil { @@ -273,7 +287,9 @@ func downloadViaTarGz(targetDir, ref string) (string, error) { defer os.Remove(f.Name()) defer f.Close() - prefix := fmt.Sprintf("auth0-agent-skills-%s/%s/", ref, pluginSubtreePath) + // GitHub flattens "/" in ref names to "-" in archive root directory names. + archiveRef := strings.ReplaceAll(ref, "/", "-") + prefix := fmt.Sprintf("auth0-agent-skills-%s/%s/", archiveRef, pluginSubtreePath) if err := extractTarGzSubtree(f, prefix, targetDir); err != nil { return "", err } @@ -296,7 +312,9 @@ func downloadViaZip(targetDir, ref string) (string, error) { defer os.Remove(f.Name()) defer f.Close() - prefix := fmt.Sprintf("auth0-agent-skills-%s/%s/", ref, pluginSubtreePath) + // GitHub flattens "/" in ref names to "-" in archive root directory names. + archiveRef := strings.ReplaceAll(ref, "/", "-") + prefix := fmt.Sprintf("auth0-agent-skills-%s/%s/", archiveRef, pluginSubtreePath) if err := extractZipSubtree(f.Name(), size, prefix, targetDir); err != nil { return "", err } @@ -304,7 +322,8 @@ func downloadViaZip(targetDir, ref string) (string, error) { return sha, nil } -// mergeDir recursively copies the contents of src into dst. +// mergeDir recursively copies the contents of src into dst. Symlinks are preserved +// (not dereferenced) so the layout matches what git sparse-checkout produces. func mergeDir(src, dst string) error { entries, err := os.ReadDir(src) if err != nil { @@ -313,14 +332,23 @@ func mergeDir(src, dst string) error { for _, entry := range entries { srcPath := filepath.Join(src, entry.Name()) dstPath := filepath.Join(dst, entry.Name()) - if entry.IsDir() { + switch { + case entry.Type()&os.ModeSymlink != 0: + target, err := os.Readlink(srcPath) + if err != nil { + return err + } + if err := os.Symlink(target, dstPath); err != nil { + return err + } + case entry.IsDir(): if err := os.MkdirAll(dstPath, 0o755); err != nil { return err } if err := mergeDir(srcPath, dstPath); err != nil { return err } - } else { + default: info, err := entry.Info() if err != nil { return err diff --git a/internal/ai/skills/download_test.go b/internal/ai/skills/download_test.go index e352ee037..ad896248a 100644 --- a/internal/ai/skills/download_test.go +++ b/internal/ai/skills/download_test.go @@ -351,24 +351,34 @@ func TestFetchToTempFile_Truncation(t *testing.T) { }) } -// --- checkNonEmpty ---. +// --- checkHasSkills ---. -func TestCheckNonEmpty(t *testing.T) { - t.Run("returns error for empty directory", func(t *testing.T) { +func TestCheckHasSkills(t *testing.T) { + t.Run("returns error when skills subdirectory is absent", func(t *testing.T) { dir := t.TempDir() - err := checkNonEmpty(dir) + err := checkHasSkills(dir) require.Error(t, err) - assert.Contains(t, err.Error(), "no files") + assert.Contains(t, err.Error(), "no skills found") }) - t.Run("returns nil for directory with at least one entry", func(t *testing.T) { + t.Run("returns error when skills subdirectory is empty", func(t *testing.T) { dir := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(dir, "file.txt"), []byte("x"), 0o644)) - assert.NoError(t, checkNonEmpty(dir)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "skills"), 0o755)) + err := checkHasSkills(dir) + require.Error(t, err) + assert.Contains(t, err.Error(), "no skills found") + }) + + t.Run("returns nil when skills subdirectory has at least one entry", func(t *testing.T) { + dir := t.TempDir() + skillDir := filepath.Join(dir, "skills", "my-skill") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("x"), 0o644)) + assert.NoError(t, checkHasSkills(dir)) }) t.Run("returns error for non-existent directory", func(t *testing.T) { - err := checkNonEmpty(filepath.Join(t.TempDir(), "does-not-exist")) + err := checkHasSkills(filepath.Join(t.TempDir(), "does-not-exist")) require.Error(t, err) }) } @@ -473,13 +483,11 @@ func TestDownloadViaZip(t *testing.T) { // --- DownloadPlugin ---. func TestDownloadPlugin_EmptyExtraction(t *testing.T) { - // When the archive contains no entries matching the expected prefix, checkNonEmpty - // should cause the strategy to fail, and DownloadPlugin returns an error. - // Skip when git is usable: DownloadPlugin will clone from the real repo and succeed, - // bypassing the HTTP mock strategies entirely. - if _, err := exec.LookPath("git"); err == nil && checkGitVersion() == nil { - t.Skip("git is available; DownloadPlugin uses git strategy which bypasses HTTP mock") - } + // Force HTTP strategies by disabling git lookup so the test is not skipped + // in environments where git >= 2.25 is available. + orig := gitLookPath + gitLookPath = func(string) (string, error) { return "", errors.New("git not found") } + t.Cleanup(func() { gitLookPath = orig }) const ref = "main" @@ -501,7 +509,6 @@ func TestDownloadPlugin_EmptyExtraction(t *testing.T) { case "github.com": return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(wrongPrefixZipData))}, nil default: - // SHA API — shouldn't be reached if extraction fails. body, _ := json.Marshal(map[string]string{"sha": "abc"}) return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(bytes.NewReader(body))}, nil } @@ -510,20 +517,27 @@ func TestDownloadPlugin_EmptyExtraction(t *testing.T) { base := t.TempDir() targetDir := filepath.Join(base, "auth0") _, err = DownloadPlugin(targetDir, ref) - // All three strategies should fail (git is skipped if not in PATH or fails network; - // tar.gz and ZIP produce empty dirs which checkNonEmpty rejects). + // tar.gz and ZIP extract nothing; checkHasSkills rejects the empty result. require.Error(t, err) + assert.Contains(t, err.Error(), "no skills found") } func TestDownloadPlugin_CreatesMissingTargetDir(t *testing.T) { + // Force HTTP strategies so the test is deterministic regardless of git availability. + orig := gitLookPath + gitLookPath = func(string) (string, error) { return "", errors.New("git not found") } + t.Cleanup(func() { gitLookPath = orig }) + const ref = "main" const wantSHA = "abc123" prefix := fmt.Sprintf("auth0-agent-skills-%s/%s/", ref, pluginSubtreePath) + // Archive paths include the skills/ subdirectory to match the real repo layout and + // satisfy checkHasSkills which verifies targetDir/skills/ is non-empty. tarData := makeTarGz(t, map[string]string{ - prefix + "skill-a/SKILL.md": "# skill-a", + prefix + "skills/skill-a/SKILL.md": "# skill-a", }) zipPath, _ := makeZip(t, map[string]string{ - prefix + "skill-a/SKILL.md": "# skill-a", + prefix + "skills/skill-a/SKILL.md": "# skill-a", }) zipData, err := os.ReadFile(zipPath) require.NoError(t, err) @@ -542,13 +556,98 @@ func TestDownloadPlugin_CreatesMissingTargetDir(t *testing.T) { // targetDir is deeply nested and does not exist yet. targetDir := filepath.Join(t.TempDir(), "deep", "nested", "auth0") - _, err = DownloadPlugin(targetDir, ref) - // If git is available and succeeds via real network, sha may differ; we just check - // that the call succeeded and targetDir was created with at least one entry. - if err == nil { - entries, readErr := os.ReadDir(targetDir) - require.NoError(t, readErr) - assert.NotEmpty(t, entries, "targetDir must contain extracted files") + gotSHA, err := DownloadPlugin(targetDir, ref) + require.NoError(t, err) + assert.Equal(t, wantSHA, gotSHA) + entries, readErr := os.ReadDir(targetDir) + require.NoError(t, readErr) + assert.NotEmpty(t, entries, "targetDir must contain extracted files") +} + +// --- downloadViaGit ---. + +// setupLocalGitRepo creates a local bare repository seeded with the given file tree under +// the specified branch. Returns the path of the bare repository. +func setupLocalGitRepo(t *testing.T, branch string, files map[string]string) string { + t.Helper() + + workDir := t.TempDir() + bareDir := t.TempDir() + + runSetup := func(dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("setup git %v: %v\n%s", args, err, out) + } + } + + runSetup(workDir, "init", "-b", branch) + runSetup(workDir, "config", "user.email", "test@example.com") + runSetup(workDir, "config", "user.name", "Test") + runSetup(workDir, "config", "commit.gpgsign", "false") + + for name, content := range files { + path := filepath.Join(workDir, filepath.FromSlash(name)) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) } - // If git fails in test environment (no network), tar.gz or ZIP mock should succeed. + + runSetup(workDir, "add", ".") + runSetup(workDir, "commit", "-m", "init") + runSetup(bareDir, "init", "--bare", "-b", branch) + runSetup(workDir, "remote", "add", "origin", bareDir) + runSetup(workDir, "push", "origin", branch) + + return bareDir +} + +func TestDownloadViaGit_Layout(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + if err := checkGitVersion(); err != nil { + t.Skip("git too old: " + err.Error()) + } + + bareDir := setupLocalGitRepo(t, "main", map[string]string{ + "plugins/auth0/skills/skill-test/SKILL.md": "---\nname: test\n---\n# Test Skill", + "plugins/auth0/skills/skill-b/SKILL.md": "---\nname: b\n---\n# B", + }) + + orig := agentSkillsGitURL + agentSkillsGitURL = bareDir + t.Cleanup(func() { agentSkillsGitURL = orig }) + + targetDir := t.TempDir() + sha, err := downloadViaGit(targetDir, "main") + require.NoError(t, err) + assert.NotEmpty(t, sha, "returned SHA should be non-empty") + + assertFileContent(t, filepath.Join(targetDir, "skills", "skill-test", "SKILL.md"), "---\nname: test\n---\n# Test Skill") + assertFileContent(t, filepath.Join(targetDir, "skills", "skill-b", "SKILL.md"), "---\nname: b\n---\n# B") +} + +func TestDownloadViaGit_CloneFailure(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + if err := checkGitVersion(); err != nil { + t.Skip("git too old: " + err.Error()) + } + + orig := agentSkillsGitURL + agentSkillsGitURL = "/nonexistent/does/not/exist" + t.Cleanup(func() { agentSkillsGitURL = orig }) + + targetDir := t.TempDir() + _, err := downloadViaGit(targetDir, "main") + require.Error(t, err, "clone of non-existent repo should fail") + + // targetDir must still exist and be empty so the caller can fall back to HTTP strategies. + entries, readErr := os.ReadDir(targetDir) + require.NoError(t, readErr, "targetDir should still exist after clone failure") + assert.Empty(t, entries, "targetDir should be empty after clone failure") } From 480b81b13d6a1509208a824aceaf2f717512880c Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Thu, 4 Jun 2026 09:28:40 +0530 Subject: [PATCH 13/23] fix: setupLocalGit test fix --- internal/ai/skills/download_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/ai/skills/download_test.go b/internal/ai/skills/download_test.go index ad896248a..fd9b62fb1 100644 --- a/internal/ai/skills/download_test.go +++ b/internal/ai/skills/download_test.go @@ -584,7 +584,8 @@ func setupLocalGitRepo(t *testing.T, branch string, files map[string]string) str } } - runSetup(workDir, "init", "-b", branch) + runSetup(workDir, "init") + runSetup(workDir, "symbolic-ref", "HEAD", "refs/heads/"+branch) runSetup(workDir, "config", "user.email", "test@example.com") runSetup(workDir, "config", "user.name", "Test") runSetup(workDir, "config", "commit.gpgsign", "false") @@ -597,7 +598,8 @@ func setupLocalGitRepo(t *testing.T, branch string, files map[string]string) str runSetup(workDir, "add", ".") runSetup(workDir, "commit", "-m", "init") - runSetup(bareDir, "init", "--bare", "-b", branch) + runSetup(bareDir, "init", "--bare") + runSetup(bareDir, "symbolic-ref", "HEAD", "refs/heads/"+branch) runSetup(workDir, "remote", "add", "origin", bareDir) runSetup(workDir, "push", "origin", branch) From bd87f9a5d46ec62cc8117b5da2901ad7b5392956 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Thu, 4 Jun 2026 13:38:09 +0530 Subject: [PATCH 14/23] fix: naming, scope enum, concurrent lockfile access inter process --- internal/ai/skills/lock.go | 57 +++++++++++++++++++++--- internal/ai/skills/lock_flock_unix.go | 21 +++++++++ internal/ai/skills/lock_flock_windows.go | 24 ++++++++++ internal/ai/skills/lock_test.go | 18 ++++---- 4 files changed, 106 insertions(+), 14 deletions(-) create mode 100644 internal/ai/skills/lock_flock_unix.go create mode 100644 internal/ai/skills/lock_flock_windows.go diff --git a/internal/ai/skills/lock.go b/internal/ai/skills/lock.go index 6dbb61a3b..89e01e83f 100644 --- a/internal/ai/skills/lock.go +++ b/internal/ai/skills/lock.go @@ -8,8 +8,19 @@ import ( "time" ) +type Scope string + +const ( + ScopeGlobal Scope = "global" + ScopeLocal Scope = "local" +) + +func (s Scope) Valid() bool { + return s == ScopeGlobal || s == ScopeLocal +} + // Lock records the installed state of the auth0 agent-skills plugin. -type Lock struct { +type SkillsLock struct { Repo string `json:"repo"` Ref string `json:"ref"` CommitSHA string `json:"commitSHA"` @@ -18,11 +29,29 @@ type Lock struct { LastCheckedAt time.Time `json:"lastCheckedAt"` Skills []string `json:"skills"` Agents []string `json:"agents"` - Scope string `json:"scope"` // "global" or "local". + Scope Scope `json:"scope"` +} + +// openFlockFile opens (or creates) the advisory lock file used to coordinate concurrent access. +func openFlockFile(path string) (*os.File, error) { + return os.OpenFile(path+".lock", os.O_CREATE|os.O_RDWR, 0o644) } // ReadLock reads the skills-lock.json at path. Returns nil, nil when the file does not exist. -func ReadLock(path string) (*Lock, error) { +// It acquires a shared file lock for the duration of the read. +func ReadLock(path string) (*SkillsLock, error) { + fl, err := openFlockFile(path) + if err != nil { + return nil, err + } + defer func() { + releaseFlock(fl) + fl.Close() + }() + if err := acquireSharedFlock(fl); err != nil { + return nil, err + } + data, err := os.ReadFile(path) if err != nil { if errors.Is(err, os.ErrNotExist) { @@ -30,7 +59,7 @@ func ReadLock(path string) (*Lock, error) { } return nil, err } - var lock Lock + var lock SkillsLock if err := json.Unmarshal(data, &lock); err != nil { return nil, err } @@ -38,10 +67,28 @@ func ReadLock(path string) (*Lock, error) { } // WriteLock serialises lock as JSON and writes it to path, creating parent directories as needed. -func WriteLock(path string, lock *Lock) error { +// It acquires an exclusive file lock for the duration of the write. +func WriteLock(path string, lock *SkillsLock) error { if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return err } + + fl, err := openFlockFile(path) + if err != nil { + return err + } + defer func() { + releaseFlock(fl) + fl.Close() + }() + if err := acquireExclusiveFlock(fl); err != nil { + return err + } + + if !lock.Scope.Valid() { + return errors.New("invalid scope: " + string(lock.Scope) + " (must be 'global' or 'local')") + } + os.Truncate(path, 0) // best-effort: ensures a failed write leaves an empty file, not stale data data, err := json.MarshalIndent(lock, "", " ") if err != nil { return err diff --git a/internal/ai/skills/lock_flock_unix.go b/internal/ai/skills/lock_flock_unix.go new file mode 100644 index 000000000..9b78c55e5 --- /dev/null +++ b/internal/ai/skills/lock_flock_unix.go @@ -0,0 +1,21 @@ +//go:build !windows + +package skills + +import ( + "os" + + "golang.org/x/sys/unix" +) + +func acquireSharedFlock(f *os.File) error { + return unix.Flock(int(f.Fd()), unix.LOCK_SH) +} + +func acquireExclusiveFlock(f *os.File) error { + return unix.Flock(int(f.Fd()), unix.LOCK_EX) +} + +func releaseFlock(f *os.File) error { + return unix.Flock(int(f.Fd()), unix.LOCK_UN) +} diff --git a/internal/ai/skills/lock_flock_windows.go b/internal/ai/skills/lock_flock_windows.go new file mode 100644 index 000000000..791ef8104 --- /dev/null +++ b/internal/ai/skills/lock_flock_windows.go @@ -0,0 +1,24 @@ +//go:build windows + +package skills + +import ( + "os" + + "golang.org/x/sys/windows" +) + +func acquireSharedFlock(f *os.File) error { + ol := new(windows.Overlapped) + return windows.LockFileEx(windows.Handle(f.Fd()), 0, 0, 1, 0, ol) +} + +func acquireExclusiveFlock(f *os.File) error { + ol := new(windows.Overlapped) + return windows.LockFileEx(windows.Handle(f.Fd()), windows.LOCKFILE_EXCLUSIVE_LOCK, 0, 1, 0, ol) +} + +func releaseFlock(f *os.File) error { + ol := new(windows.Overlapped) + return windows.UnlockFileEx(windows.Handle(f.Fd()), 0, 1, 0, ol) +} diff --git a/internal/ai/skills/lock_test.go b/internal/ai/skills/lock_test.go index 5c50213aa..81ec4523b 100644 --- a/internal/ai/skills/lock_test.go +++ b/internal/ai/skills/lock_test.go @@ -41,7 +41,7 @@ func TestReadLock(t *testing.T) { assert.Equal(t, "abc123", lock.CommitSHA) assert.Equal(t, []string{"auth0-react", "auth0-nextjs"}, lock.Skills) assert.Equal(t, []string{"claude-code"}, lock.Agents) - assert.Equal(t, "global", lock.Scope) + assert.Equal(t, ScopeGlobal, lock.Scope) }) t.Run("returns error for invalid JSON", func(t *testing.T) { @@ -74,7 +74,7 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "skills-lock.json") - lock := &Lock{ + lock := &SkillsLock{ Repo: "https://github.com/auth0/agent-skills.git", Ref: "main", CommitSHA: "deadbeef", @@ -83,7 +83,7 @@ func TestWriteLock(t *testing.T) { LastCheckedAt: now, Skills: []string{"auth0-react"}, Agents: []string{"cursor"}, - Scope: "local", + Scope: ScopeLocal, } require.NoError(t, WriteLock(path, lock)) @@ -102,20 +102,20 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "nested", "deep", "skills-lock.json") - require.NoError(t, WriteLock(path, &Lock{Scope: "global"})) + require.NoError(t, WriteLock(path, &SkillsLock{Scope: ScopeGlobal})) got, err := ReadLock(path) require.NoError(t, err) require.NotNil(t, got) - assert.Equal(t, "global", got.Scope) + assert.Equal(t, ScopeGlobal, got.Scope) }) t.Run("overwrites existing lock file", func(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "skills-lock.json") - require.NoError(t, WriteLock(path, &Lock{CommitSHA: "first"})) - require.NoError(t, WriteLock(path, &Lock{CommitSHA: "second"})) + require.NoError(t, WriteLock(path, &SkillsLock{CommitSHA: "first", Scope: ScopeGlobal})) + require.NoError(t, WriteLock(path, &SkillsLock{CommitSHA: "second", Scope: ScopeGlobal})) got, err := ReadLock(path) require.NoError(t, err) @@ -126,7 +126,7 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "skills-lock.json") - original := &Lock{ + original := &SkillsLock{ Repo: "https://github.com/auth0/agent-skills.git", Ref: "v1.2.3", CommitSHA: "cafebabe", @@ -135,7 +135,7 @@ func TestWriteLock(t *testing.T) { LastCheckedAt: now.Add(2 * time.Hour), Skills: []string{"auth0-react", "auth0-nextjs", "auth0-vue"}, Agents: []string{"claude-code", "cursor", "gemini-cli"}, - Scope: "global", + Scope: ScopeGlobal, } require.NoError(t, WriteLock(path, original)) From 7bca622a7f7a226e92cad7e51d34a9f6a2cb6a2a Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Thu, 4 Jun 2026 15:07:23 +0530 Subject: [PATCH 15/23] fix: skills meta parsing fix, available skills nil fix, skills.md file read limit --- internal/ai/skills/skill_meta.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/internal/ai/skills/skill_meta.go b/internal/ai/skills/skill_meta.go index ddf757e74..7121f2039 100644 --- a/internal/ai/skills/skill_meta.go +++ b/internal/ai/skills/skill_meta.go @@ -1,10 +1,11 @@ package skills import ( + "io" "os" "path/filepath" + "regexp" "sort" - "strings" "gopkg.in/yaml.v3" ) @@ -18,12 +19,21 @@ type SkillMeta struct { // ParseSkillMeta reads SKILL.md from skillDir and extracts the YAML frontmatter. // Returns an empty SkillMeta (no error) when the file has no valid frontmatter delimiters. func ParseSkillMeta(skillDir string) (SkillMeta, error) { - data, err := os.ReadFile(filepath.Join(skillDir, "SKILL.md")) + f, err := os.Open(filepath.Join(skillDir, "SKILL.md")) + + if err != nil { + return SkillMeta{}, err + } + defer f.Close() + + data, err := io.ReadAll(io.LimitReader(f, 1024*1024)) + if err != nil { return SkillMeta{}, err } - parts := strings.SplitN(string(data), "---", 3) + re := regexp.MustCompile(`(?m)^---[ \t]*$`) + parts := re.Split(string(data), 3) if len(parts) < 3 { return SkillMeta{}, nil } @@ -39,11 +49,11 @@ func ParseSkillMeta(skillDir string) (SkillMeta, error) { // subdirectory that contains a valid SKILL.md, sorted alphabetically by name. func ListAvailableSkills(pluginSkillsDir string) ([]SkillMeta, error) { entries, err := os.ReadDir(pluginSkillsDir) + var skills []SkillMeta if err != nil { - return nil, err + return skills, err } - var skills []SkillMeta for _, entry := range entries { if !entry.IsDir() { continue From 8bcbd7180a7f9730e0f3160db7f42e78255d02df Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Thu, 4 Jun 2026 16:02:35 +0530 Subject: [PATCH 16/23] fix: removed quickstart related file changes --- internal/auth0/quickstart.go | 2 +- internal/cli/quickstart_detect.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/auth0/quickstart.go b/internal/auth0/quickstart.go index 7e909f603..daf2360f9 100644 --- a/internal/auth0/quickstart.go +++ b/internal/auth0/quickstart.go @@ -599,7 +599,7 @@ var QuickstartConfigs = map[string]AppConfig{ AllowedLogoutURLs: []string{DetectionSub}, Name: DetectionSub, // Okta-spring-boot-starter registers redirect under "oidc" registration ID. - CallbackPath: "/login/oauth2/code/oidc", + CallbackPath: "/login/oauth2/code/okta", }, Strategy: FileOutputStrategy{Path: "src/main/resources/application.yml", Format: "yaml"}, }, diff --git a/internal/cli/quickstart_detect.go b/internal/cli/quickstart_detect.go index 889097f30..d2f82d0b8 100644 --- a/internal/cli/quickstart_detect.go +++ b/internal/cli/quickstart_detect.go @@ -35,7 +35,7 @@ func DetectProject(dir string) DetectionResult { // Read package.json deps early - needed for checks that must precede file-based signals. earlyDeps := readPackageJSONDeps(dir) - // -- 1. Manage.py (Django) — checked before Ionic to prevent monorepo misdetection. + // -- 1. Manage.py (Django) — Checked before Ionic to prevent monorepo misdetection. if fileExists(dir, "manage.py") { result.Framework = "django" result.Type = "regular" @@ -80,7 +80,7 @@ func DetectProject(dir string) DetectionResult { if data, ok := readFileContent(dir, "pubspec.yaml"); ok { if strings.Contains(data, "sdk: flutter") { result.Detected = true - // Android/ios dirs signal native; flutter.web key signals web SPA; default native. + // Android/iOS dirs signal native; flutter.web key signals web SPA; default native. switch { case dirExists(dir, "android") || dirExists(dir, "ios"): result.Framework = "flutter" @@ -99,7 +99,7 @@ func DetectProject(dir string) DetectionResult { } } - // -- 5. Composer.json (PHP) — before vite.config to avoid Laravel misdetection. + // -- 5. Composer.json (PHP) — Before vite.config to avoid Laravel misdetection. if data, ok := readFileContent(dir, "composer.json"); ok { result.BuildTool = "composer" result.Type = "regular" @@ -121,7 +121,7 @@ func DetectProject(dir string) DetectionResult { return result } - // -- 7. Nuxt.config.[ts|js] — before vite.config (Nuxt uses Vite internally). + // -- 7. Nuxt.config.[ts|js] — Before vite.config (Nuxt uses Vite internally). if fileExistsAny(dir, "nuxt.config.ts", "nuxt.config.js") { result.Framework = "nuxt" result.Type = "regular" @@ -155,7 +155,7 @@ func DetectProject(dir string) DetectionResult { return result } - // -- 10. Svelte.config.[js|ts] — only label as sveltekit when @sveltejs/kit dep is confirmed. + // -- 10. Svelte.config.[js|ts] — Only label as sveltekit when @sveltejs/kit dep is confirmed. if fileExistsAny(dir, "svelte.config.js", "svelte.config.ts") { framework := "sveltekit" appType := "regular" @@ -203,7 +203,7 @@ func DetectProject(dir string) DetectionResult { return result } - // -- 14. IOS Swift — .xcodeproj or Package.swift (excludes Vapor server-side Swift). + // -- 14. IOS Swift — .xcodeproj or Package.swift (Excludes Vapor server-side Swift). if hasXcodeprojDir(dir) || (fileExists(dir, "Package.swift") && !isVaporSwiftPackage(dir)) { result.Framework = "ios-swift" result.Type = "native" From 0da65371b874b9e0a50fb9d7d45f54d253dbaa10 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Thu, 4 Jun 2026 16:43:41 +0530 Subject: [PATCH 17/23] fix: lint fixes --- .gitignore | 4 +++- internal/ai/skills/agent_test.go | 10 +++++----- internal/ai/skills/download.go | 21 ++++++++++----------- internal/ai/skills/download_test.go | 8 ++++---- internal/ai/skills/lock.go | 14 +++++++------- internal/ai/skills/lock_test.go | 10 +++++----- internal/ai/skills/skill_meta_test.go | 2 +- 7 files changed, 35 insertions(+), 34 deletions(-) diff --git a/.gitignore b/.gitignore index 73c652f17..959684827 100644 --- a/.gitignore +++ b/.gitignore @@ -45,4 +45,6 @@ out/ .github/copilot-instructions.md # LLM files -.remember/ \ No newline at end of file +.remember/ + +.gopath diff --git a/internal/ai/skills/agent_test.go b/internal/ai/skills/agent_test.go index 7561eaf4b..3cd7fcec3 100644 --- a/internal/ai/skills/agent_test.go +++ b/internal/ai/skills/agent_test.go @@ -213,7 +213,7 @@ func TestSupportedAgents(t *testing.T) { }) t.Run("agents with no detection are detectable-never", func(t *testing.T) { - // openhands, trae, mux, and universal have nil markers/binaries meaning IsInstalled + // Openhands, trae, mux, and universal have nil markers/binaries meaning IsInstalled // always returns false; they are included via explicit ID checks or --agent flag. noDetectIDs := []string{"openhands", "trae", "mux", "universal"} byID := make(map[string]AgentConfig) @@ -310,11 +310,11 @@ func TestResetDetectedAgentsCache(t *testing.T) { // Temporarily inject a fake agent that detects a temp dir. dir := t.TempDir() fake := AgentConfig{ - ID: "test-reset-agent", - DisplayName: "Test Reset Agent", - GlobalSkillsDir: filepath.Join(dir, "skills"), + ID: "test-reset-agent", + DisplayName: "Test Reset Agent", + GlobalSkillsDir: filepath.Join(dir, "skills"), ProjectSkillsDir: filepath.Join(".agents", "skills"), - DetectMarkers: []string{filepath.Join(dir, "marker")}, + DetectMarkers: []string{filepath.Join(dir, "marker")}, } original := SupportedAgents t.Cleanup(func() { diff --git a/internal/ai/skills/download.go b/internal/ai/skills/download.go index 2c88c139e..e59662f01 100644 --- a/internal/ai/skills/download.go +++ b/internal/ai/skills/download.go @@ -53,27 +53,27 @@ func DownloadPlugin(targetDir, ref string) (string, error) { if err != nil { return "", fmt.Errorf("create temp dir: %w", err) } - defer os.RemoveAll(tmpDir) // no-op if renamed to targetDir below + defer os.RemoveAll(tmpDir) // No-op if renamed to targetDir below. sha, dlErr := func() (string, error) { var errs []string if _, err := gitLookPath("git"); err == nil && checkGitVersion() == nil { - if sha, err := downloadViaGit(tmpDir, ref); err == nil { + sha, err := downloadViaGit(tmpDir, ref) + if err == nil { return sha, nil - } else { - errs = append(errs, "git: "+err.Error()) } + errs = append(errs, "git: "+err.Error()) } - if sha, err := downloadViaTarGz(tmpDir, ref); err == nil { + sha, err := downloadViaTarGz(tmpDir, ref) + if err == nil { return sha, nil - } else { - errs = append(errs, "tar.gz: "+err.Error()) } - if sha, err := downloadViaZip(tmpDir, ref); err == nil { + errs = append(errs, "tar.gz: "+err.Error()) + sha, err = downloadViaZip(tmpDir, ref) + if err == nil { return sha, nil - } else { - errs = append(errs, "zip: "+err.Error()) } + errs = append(errs, "zip: "+err.Error()) return "", fmt.Errorf("all download strategies failed: %s", strings.Join(errs, "; ")) }() if dlErr != nil { @@ -469,4 +469,3 @@ func extractZipSubtree(zipPath string, size int64, prefix, destDir string) error } return nil } - diff --git a/internal/ai/skills/download_test.go b/internal/ai/skills/download_test.go index fd9b62fb1..2c60478e1 100644 --- a/internal/ai/skills/download_test.go +++ b/internal/ai/skills/download_test.go @@ -340,7 +340,7 @@ func TestFetchToTempFile_Truncation(t *testing.T) { maxSkillsDownload = 10 t.Cleanup(func() { maxSkillsDownload = orig }) - body := strings.Repeat("x", 9) // 9 < 10 + body := strings.Repeat("x", 9) // 9 < 10. setHTTPClient(t, func(_ *http.Request) (*http.Response, error) { return &http.Response{StatusCode: http.StatusOK, Body: io.NopCloser(strings.NewReader(body))}, nil }) @@ -517,7 +517,7 @@ func TestDownloadPlugin_EmptyExtraction(t *testing.T) { base := t.TempDir() targetDir := filepath.Join(base, "auth0") _, err = DownloadPlugin(targetDir, ref) - // tar.gz and ZIP extract nothing; checkHasSkills rejects the empty result. + // Tar.gz and ZIP extract nothing; checkHasSkills rejects the empty result. require.Error(t, err) assert.Contains(t, err.Error(), "no skills found") } @@ -554,7 +554,7 @@ func TestDownloadPlugin_CreatesMissingTargetDir(t *testing.T) { } }) - // targetDir is deeply nested and does not exist yet. + // TargetDir is deeply nested and does not exist yet. targetDir := filepath.Join(t.TempDir(), "deep", "nested", "auth0") gotSHA, err := DownloadPlugin(targetDir, ref) require.NoError(t, err) @@ -648,7 +648,7 @@ func TestDownloadViaGit_CloneFailure(t *testing.T) { _, err := downloadViaGit(targetDir, "main") require.Error(t, err, "clone of non-existent repo should fail") - // targetDir must still exist and be empty so the caller can fall back to HTTP strategies. + // TargetDir must still exist and be empty so the caller can fall back to HTTP strategies. entries, readErr := os.ReadDir(targetDir) require.NoError(t, readErr, "targetDir should still exist after clone failure") assert.Empty(t, entries, "targetDir should be empty after clone failure") diff --git a/internal/ai/skills/lock.go b/internal/ai/skills/lock.go index 89e01e83f..15935c381 100644 --- a/internal/ai/skills/lock.go +++ b/internal/ai/skills/lock.go @@ -20,7 +20,7 @@ func (s Scope) Valid() bool { } // Lock records the installed state of the auth0 agent-skills plugin. -type SkillsLock struct { +type Lock struct { Repo string `json:"repo"` Ref string `json:"ref"` CommitSHA string `json:"commitSHA"` @@ -39,13 +39,13 @@ func openFlockFile(path string) (*os.File, error) { // ReadLock reads the skills-lock.json at path. Returns nil, nil when the file does not exist. // It acquires a shared file lock for the duration of the read. -func ReadLock(path string) (*SkillsLock, error) { +func ReadLock(path string) (*Lock, error) { fl, err := openFlockFile(path) if err != nil { return nil, err } defer func() { - releaseFlock(fl) + _ = releaseFlock(fl) fl.Close() }() if err := acquireSharedFlock(fl); err != nil { @@ -59,7 +59,7 @@ func ReadLock(path string) (*SkillsLock, error) { } return nil, err } - var lock SkillsLock + var lock Lock if err := json.Unmarshal(data, &lock); err != nil { return nil, err } @@ -68,7 +68,7 @@ func ReadLock(path string) (*SkillsLock, error) { // WriteLock serialises lock as JSON and writes it to path, creating parent directories as needed. // It acquires an exclusive file lock for the duration of the write. -func WriteLock(path string, lock *SkillsLock) error { +func WriteLock(path string, lock *Lock) error { if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return err } @@ -78,7 +78,7 @@ func WriteLock(path string, lock *SkillsLock) error { return err } defer func() { - releaseFlock(fl) + _ = releaseFlock(fl) fl.Close() }() if err := acquireExclusiveFlock(fl); err != nil { @@ -88,7 +88,7 @@ func WriteLock(path string, lock *SkillsLock) error { if !lock.Scope.Valid() { return errors.New("invalid scope: " + string(lock.Scope) + " (must be 'global' or 'local')") } - os.Truncate(path, 0) // best-effort: ensures a failed write leaves an empty file, not stale data + os.Truncate(path, 0) // Best-effort: ensures a failed write leaves an empty file, not stale data. data, err := json.MarshalIndent(lock, "", " ") if err != nil { return err diff --git a/internal/ai/skills/lock_test.go b/internal/ai/skills/lock_test.go index 81ec4523b..e7eb6397d 100644 --- a/internal/ai/skills/lock_test.go +++ b/internal/ai/skills/lock_test.go @@ -74,7 +74,7 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "skills-lock.json") - lock := &SkillsLock{ + lock := &Lock{ Repo: "https://github.com/auth0/agent-skills.git", Ref: "main", CommitSHA: "deadbeef", @@ -102,7 +102,7 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "nested", "deep", "skills-lock.json") - require.NoError(t, WriteLock(path, &SkillsLock{Scope: ScopeGlobal})) + require.NoError(t, WriteLock(path, &Lock{Scope: ScopeGlobal})) got, err := ReadLock(path) require.NoError(t, err) @@ -114,8 +114,8 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "skills-lock.json") - require.NoError(t, WriteLock(path, &SkillsLock{CommitSHA: "first", Scope: ScopeGlobal})) - require.NoError(t, WriteLock(path, &SkillsLock{CommitSHA: "second", Scope: ScopeGlobal})) + require.NoError(t, WriteLock(path, &Lock{CommitSHA: "first", Scope: ScopeGlobal})) + require.NoError(t, WriteLock(path, &Lock{CommitSHA: "second", Scope: ScopeGlobal})) got, err := ReadLock(path) require.NoError(t, err) @@ -126,7 +126,7 @@ func TestWriteLock(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "skills-lock.json") - original := &SkillsLock{ + original := &Lock{ Repo: "https://github.com/auth0/agent-skills.git", Ref: "v1.2.3", CommitSHA: "cafebabe", diff --git a/internal/ai/skills/skill_meta_test.go b/internal/ai/skills/skill_meta_test.go index 03b4c0bb1..9e93b6947 100644 --- a/internal/ai/skills/skill_meta_test.go +++ b/internal/ai/skills/skill_meta_test.go @@ -62,7 +62,7 @@ func TestParseSkillMeta(t *testing.T) { dir := t.TempDir() skillDir := filepath.Join(dir, "bad-yaml") require.NoError(t, os.MkdirAll(skillDir, 0o755)) - // Indentation error creates invalid YAML + // Indentation error creates invalid YAML. content := "---\nname: foo\n bad: indent: here\n---\n" require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644)) From e33852913f301229db9c0f4d3e21bfa468f7c255 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Thu, 4 Jun 2026 16:49:40 +0530 Subject: [PATCH 18/23] fix: removed quickstart comment and gitignore update --- .gitignore | 1 - internal/auth0/quickstart.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 959684827..2e4614985 100644 --- a/.gitignore +++ b/.gitignore @@ -47,4 +47,3 @@ out/ # LLM files .remember/ -.gopath diff --git a/internal/auth0/quickstart.go b/internal/auth0/quickstart.go index daf2360f9..d8a83bcda 100644 --- a/internal/auth0/quickstart.go +++ b/internal/auth0/quickstart.go @@ -598,8 +598,7 @@ var QuickstartConfigs = map[string]AppConfig{ Callbacks: []string{DetectionSub}, AllowedLogoutURLs: []string{DetectionSub}, Name: DetectionSub, - // Okta-spring-boot-starter registers redirect under "oidc" registration ID. - CallbackPath: "/login/oauth2/code/okta", + CallbackPath: "/login/oauth2/code/okta", }, Strategy: FileOutputStrategy{Path: "src/main/resources/application.yml", Format: "yaml"}, }, From 8059ad7b4698fe2ab28adf8f5df413ac860c6276 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Thu, 4 Jun 2026 19:00:24 +0530 Subject: [PATCH 19/23] feat: symlink internal init --- internal/ai/skills/symlink.go | 138 +++++++++++++++++ internal/ai/skills/symlink_test.go | 231 +++++++++++++++++++++++++++++ 2 files changed, 369 insertions(+) create mode 100644 internal/ai/skills/symlink.go create mode 100644 internal/ai/skills/symlink_test.go diff --git a/internal/ai/skills/symlink.go b/internal/ai/skills/symlink.go new file mode 100644 index 000000000..d05f56c30 --- /dev/null +++ b/internal/ai/skills/symlink.go @@ -0,0 +1,138 @@ +package skills + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" +) + +// CreateSkillLink installs skillName from sourceSkillDir into agentSkillsDir. +// useCopy=true copies files recursively; useCopy=false creates a symlink. +// The operation is idempotent: a correct existing symlink or copy is left unchanged. +func CreateSkillLink(sourceSkillDir, agentSkillsDir, skillName string, useCopy bool) error { + if err := os.MkdirAll(agentSkillsDir, 0o755); err != nil { + return fmt.Errorf("create agent skills dir: %w", err) + } + + linkPath := filepath.Join(agentSkillsDir, skillName) + + info, err := os.Lstat(linkPath) + if err == nil { + if info.Mode()&os.ModeSymlink != 0 { + if isSymlinkCorrect(linkPath, agentSkillsDir, sourceSkillDir) { + return nil + } + if rmErr := os.Remove(linkPath); rmErr != nil { + return fmt.Errorf("remove existing symlink %s: %w", linkPath, rmErr) + } + } else if info.IsDir() { + // Prior --copy install. Skip: replacing a real dir silently is unsafe. + return nil + } + } else if !os.IsNotExist(err) { + return fmt.Errorf("lstat %s: %w", linkPath, err) + } + + if useCopy { + return copyDir(sourceSkillDir, linkPath) + } + return createSymlink(sourceSkillDir, agentSkillsDir, linkPath) +} + +// isSymlinkCorrect returns true if linkPath is a non-broken symlink resolving to sourceSkillDir. +func isSymlinkCorrect(linkPath, agentSkillsDir, sourceSkillDir string) bool { + target, err := os.Readlink(linkPath) + if err != nil { + return false + } + if !filepath.IsAbs(target) { + target = filepath.Join(agentSkillsDir, target) + } + if filepath.Clean(target) != filepath.Clean(sourceSkillDir) { + return false + } + _, err = os.Stat(linkPath) + return err == nil +} + +// createSymlink creates a symlink at linkPath pointing to sourceSkillDir. +// On Unix a relative path is used. On Windows an absolute symlink is tried first, +// then a directory junction, then a file copy with a warning written to stderr. +func createSymlink(sourceSkillDir, agentSkillsDir, linkPath string) error { + if runtime.GOOS != "windows" { + rel, err := filepath.Rel(agentSkillsDir, sourceSkillDir) + if err != nil { + rel = sourceSkillDir + } + return os.Symlink(rel, linkPath) + } + + // Windows: absolute symlink → junction → copy fallback. + if err := os.Symlink(sourceSkillDir, linkPath); err == nil { + return nil + } + if err := exec.Command("cmd", "/C", "mklink", "/J", linkPath, sourceSkillDir).Run(); err == nil { + return nil + } + fmt.Fprintf(os.Stderr, "warning: symlink and junction unavailable; copying %s to %s\n", sourceSkillDir, linkPath) + return copyDir(sourceSkillDir, linkPath) +} + +// copyDir recursively copies src into a newly created dst directory. +func copyDir(src, dst string) error { + if err := os.MkdirAll(dst, 0o755); err != nil { + return fmt.Errorf("create copy dir: %w", err) + } + return mergeDir(src, dst) +} + +// RemoveSkillLink removes the skill entry (symlink or copied directory) at agentSkillsDir/skillName. +// Returns nil if the entry does not exist. +func RemoveSkillLink(agentSkillsDir, skillName string) error { + linkPath := filepath.Join(agentSkillsDir, skillName) + info, err := os.Lstat(linkPath) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return fmt.Errorf("lstat %s: %w", linkPath, err) + } + if info.Mode()&os.ModeSymlink != 0 { + return os.Remove(linkPath) + } + return os.RemoveAll(linkPath) +} + +// CheckSkillLink reports the installation state of agentSkillsDir/skillName. +// Returns: "ok", "missing", "broken", "wrong_target", or "copy". +func CheckSkillLink(agentSkillsDir, skillName, expectedSourceDir string) string { + linkPath := filepath.Join(agentSkillsDir, skillName) + info, err := os.Lstat(linkPath) + if err != nil { + return "missing" + } + + if info.Mode()&os.ModeSymlink == 0 { + return "copy" + } + + // It's a symlink. Verify the target exists. + if _, err := os.Stat(linkPath); err != nil { + return "broken" + } + + // Target exists. Check if it points to the expected place. + target, err := os.Readlink(linkPath) + if err != nil { + return "broken" + } + if !filepath.IsAbs(target) { + target = filepath.Join(agentSkillsDir, target) + } + if filepath.Clean(target) == filepath.Clean(expectedSourceDir) { + return "ok" + } + return "wrong_target" +} diff --git a/internal/ai/skills/symlink_test.go b/internal/ai/skills/symlink_test.go new file mode 100644 index 000000000..15c80b640 --- /dev/null +++ b/internal/ai/skills/symlink_test.go @@ -0,0 +1,231 @@ +package skills + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// makeSkillSource creates a temporary directory with a SKILL.md file inside. +func makeSkillSource(t *testing.T) string { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("# skill"), 0o644)) + return dir +} + +// --- CheckSkillLink --- + +func TestCheckSkillLink(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink tests skipped on windows") + } + + t.Run("missing when nothing exists", func(t *testing.T) { + agentDir := t.TempDir() + assert.Equal(t, "missing", CheckSkillLink(agentDir, "my-skill", "/some/source")) + }) + + t.Run("ok for correct relative symlink", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + rel, err := filepath.Rel(agentDir, src) + require.NoError(t, err) + require.NoError(t, os.Symlink(rel, filepath.Join(agentDir, "my-skill"))) + + assert.Equal(t, "ok", CheckSkillLink(agentDir, "my-skill", src)) + }) + + t.Run("ok for correct absolute symlink", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + require.NoError(t, os.Symlink(src, filepath.Join(agentDir, "my-skill"))) + + assert.Equal(t, "ok", CheckSkillLink(agentDir, "my-skill", src)) + }) + + t.Run("broken for dangling symlink", func(t *testing.T) { + agentDir := t.TempDir() + require.NoError(t, os.Symlink("/nonexistent/path/does/not/exist", filepath.Join(agentDir, "my-skill"))) + + assert.Equal(t, "broken", CheckSkillLink(agentDir, "my-skill", "/nonexistent/path/does/not/exist")) + }) + + t.Run("wrong_target for symlink pointing elsewhere", func(t *testing.T) { + src1 := makeSkillSource(t) + src2 := makeSkillSource(t) + agentDir := t.TempDir() + rel, err := filepath.Rel(agentDir, src1) + require.NoError(t, err) + require.NoError(t, os.Symlink(rel, filepath.Join(agentDir, "my-skill"))) + + assert.Equal(t, "wrong_target", CheckSkillLink(agentDir, "my-skill", src2)) + }) + + t.Run("copy for real directory", func(t *testing.T) { + agentDir := t.TempDir() + linkPath := filepath.Join(agentDir, "my-skill") + require.NoError(t, os.MkdirAll(linkPath, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(linkPath, "SKILL.md"), []byte("# skill"), 0o644)) + + assert.Equal(t, "copy", CheckSkillLink(agentDir, "my-skill", "/any/source")) + }) +} + +// --- CreateSkillLink --- + +func TestCreateSkillLink(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink tests skipped on windows") + } + + t.Run("creates symlink for new install", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", false)) + + assert.Equal(t, "ok", CheckSkillLink(agentDir, "my-skill", src)) + info, err := os.Lstat(filepath.Join(agentDir, "my-skill")) + require.NoError(t, err) + assert.NotZero(t, info.Mode()&os.ModeSymlink, "entry should be a symlink") + }) + + t.Run("uses relative symlink target", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", false)) + + target, err := os.Readlink(filepath.Join(agentDir, "my-skill")) + require.NoError(t, err) + assert.False(t, filepath.IsAbs(target), "symlink target should be relative, got: %s", target) + }) + + t.Run("idempotent when correct symlink already exists", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", false)) + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", false)) + + assert.Equal(t, "ok", CheckSkillLink(agentDir, "my-skill", src)) + }) + + t.Run("replaces broken symlink", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + require.NoError(t, os.Symlink("/nonexistent/path/does/not/exist", filepath.Join(agentDir, "my-skill"))) + + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", false)) + + assert.Equal(t, "ok", CheckSkillLink(agentDir, "my-skill", src)) + }) + + t.Run("replaces wrong-target symlink", func(t *testing.T) { + src1 := makeSkillSource(t) + src2 := makeSkillSource(t) + agentDir := t.TempDir() + + require.NoError(t, CreateSkillLink(src1, agentDir, "my-skill", false)) + require.NoError(t, CreateSkillLink(src2, agentDir, "my-skill", false)) + + assert.Equal(t, "ok", CheckSkillLink(agentDir, "my-skill", src2)) + }) + + t.Run("creates agent skills dir when missing", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := filepath.Join(t.TempDir(), "deep", "nested", "agent") + + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", false)) + + assert.Equal(t, "ok", CheckSkillLink(agentDir, "my-skill", src)) + }) + + t.Run("copies directory when useCopy is true", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", true)) + + assert.Equal(t, "copy", CheckSkillLink(agentDir, "my-skill", src)) + data, err := os.ReadFile(filepath.Join(agentDir, "my-skill", "SKILL.md")) + require.NoError(t, err) + assert.Equal(t, "# skill", string(data)) + }) + + t.Run("idempotent when copy already exists and useCopy is true", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", true)) + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", true)) + + assert.Equal(t, "copy", CheckSkillLink(agentDir, "my-skill", src)) + }) + + t.Run("skips real directory when useCopy is false", func(t *testing.T) { + agentDir := t.TempDir() + linkPath := filepath.Join(agentDir, "my-skill") + require.NoError(t, os.MkdirAll(linkPath, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(linkPath, "SKILL.md"), []byte("original"), 0o644)) + + src := makeSkillSource(t) + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", false)) + + data, err := os.ReadFile(filepath.Join(linkPath, "SKILL.md")) + require.NoError(t, err) + assert.Equal(t, "original", string(data), "original directory should be preserved") + }) +} + +// --- RemoveSkillLink --- + +func TestRemoveSkillLink(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink tests skipped on windows") + } + + t.Run("removes symlink", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", false)) + + require.NoError(t, RemoveSkillLink(agentDir, "my-skill")) + + assert.Equal(t, "missing", CheckSkillLink(agentDir, "my-skill", src)) + }) + + t.Run("removes copied directory", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", true)) + + require.NoError(t, RemoveSkillLink(agentDir, "my-skill")) + + assert.Equal(t, "missing", CheckSkillLink(agentDir, "my-skill", src)) + }) + + t.Run("returns nil for non-existent entry", func(t *testing.T) { + agentDir := t.TempDir() + require.NoError(t, RemoveSkillLink(agentDir, "nonexistent")) + }) + + t.Run("removes nested copied directory recursively", func(t *testing.T) { + src := t.TempDir() + nested := filepath.Join(src, "sub") + require.NoError(t, os.MkdirAll(nested, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(nested, "file.txt"), []byte("x"), 0o644)) + + agentDir := t.TempDir() + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", true)) + require.NoError(t, RemoveSkillLink(agentDir, "my-skill")) + + _, err := os.Lstat(filepath.Join(agentDir, "my-skill")) + assert.True(t, os.IsNotExist(err)) + }) +} From 689cc3562085bac7f4b6a66b167c48924ad584e5 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Sun, 7 Jun 2026 23:32:05 +0530 Subject: [PATCH 20/23] fix: symlink review agent fixes --- internal/ai/skills/download.go | 61 --------------------------- internal/ai/skills/fs_util.go | 68 ++++++++++++++++++++++++++++++ internal/ai/skills/symlink.go | 47 ++++++++++++++++++--- internal/ai/skills/symlink_test.go | 49 ++++++++++++++++++++- 4 files changed, 156 insertions(+), 69 deletions(-) create mode 100644 internal/ai/skills/fs_util.go diff --git a/internal/ai/skills/download.go b/internal/ai/skills/download.go index e59662f01..d568bb5ac 100644 --- a/internal/ai/skills/download.go +++ b/internal/ai/skills/download.go @@ -322,67 +322,6 @@ func downloadViaZip(targetDir, ref string) (string, error) { return sha, nil } -// mergeDir recursively copies the contents of src into dst. Symlinks are preserved -// (not dereferenced) so the layout matches what git sparse-checkout produces. -func mergeDir(src, dst string) error { - entries, err := os.ReadDir(src) - if err != nil { - return err - } - for _, entry := range entries { - srcPath := filepath.Join(src, entry.Name()) - dstPath := filepath.Join(dst, entry.Name()) - switch { - case entry.Type()&os.ModeSymlink != 0: - target, err := os.Readlink(srcPath) - if err != nil { - return err - } - if err := os.Symlink(target, dstPath); err != nil { - return err - } - case entry.IsDir(): - if err := os.MkdirAll(dstPath, 0o755); err != nil { - return err - } - if err := mergeDir(srcPath, dstPath); err != nil { - return err - } - default: - info, err := entry.Info() - if err != nil { - return err - } - if err := copyFile(srcPath, dstPath, info.Mode()); err != nil { - return err - } - } - } - return nil -} - -// copyFile copies src to dst with the given permission mode. -func copyFile(src, dst string, mode os.FileMode) error { - if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { - return err - } - in, err := os.Open(src) - if err != nil { - return err - } - defer in.Close() - out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, mode) - if err != nil { - return err - } - _, copyErr := io.Copy(out, in) - closeErr := out.Close() - if copyErr != nil { - return copyErr - } - return closeErr -} - // ExtractEntry writes a single archive entry to destDir. IsDir and mode describe the entry; // open returns a reader for its content (ignored when isDir is true). The name is checked // against prefix and any path-traversal attempt is rejected. diff --git a/internal/ai/skills/fs_util.go b/internal/ai/skills/fs_util.go new file mode 100644 index 000000000..025d13d2c --- /dev/null +++ b/internal/ai/skills/fs_util.go @@ -0,0 +1,68 @@ +package skills + +import ( + "io" + "os" + "path/filepath" +) + +// mergeDir recursively copies the contents of src into dst. Symlinks are preserved +// (not dereferenced) so the layout matches what git sparse-checkout produces. +func mergeDir(src, dst string) error { + entries, err := os.ReadDir(src) + if err != nil { + return err + } + for _, entry := range entries { + srcPath := filepath.Join(src, entry.Name()) + dstPath := filepath.Join(dst, entry.Name()) + switch { + case entry.Type()&os.ModeSymlink != 0: + target, err := os.Readlink(srcPath) + if err != nil { + return err + } + if err := os.Symlink(target, dstPath); err != nil { + return err + } + case entry.IsDir(): + if err := os.MkdirAll(dstPath, 0o755); err != nil { + return err + } + if err := mergeDir(srcPath, dstPath); err != nil { + return err + } + default: + info, err := entry.Info() + if err != nil { + return err + } + if err := copyFile(srcPath, dstPath, info.Mode()); err != nil { + return err + } + } + } + return nil +} + +// copyFile copies src to dst with the given permission mode. +func copyFile(src, dst string, mode os.FileMode) error { + if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil { + return err + } + in, err := os.Open(src) + if err != nil { + return err + } + defer in.Close() + out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, mode) + if err != nil { + return err + } + _, copyErr := io.Copy(out, in) + closeErr := out.Close() + if copyErr != nil { + return copyErr + } + return closeErr +} diff --git a/internal/ai/skills/symlink.go b/internal/ai/skills/symlink.go index d05f56c30..8f741e4ab 100644 --- a/internal/ai/skills/symlink.go +++ b/internal/ai/skills/symlink.go @@ -28,8 +28,15 @@ func CreateSkillLink(sourceSkillDir, agentSkillsDir, skillName string, useCopy b return fmt.Errorf("remove existing symlink %s: %w", linkPath, rmErr) } } else if info.IsDir() { - // Prior --copy install. Skip: replacing a real dir silently is unsafe. - return nil + if !useCopy { + fmt.Fprintf(os.Stderr, + "warning: %s is a copied directory; remove it manually to switch to symlink mode\n", + linkPath) + return nil + } + // useCopy=true: fall through to re-copy with replace semantics. + } else { + return fmt.Errorf("%s exists as a regular file; remove it before installing skill %q", linkPath, skillName) } } else if !os.IsNotExist(err) { return fmt.Errorf("lstat %s: %w", linkPath, err) @@ -80,12 +87,35 @@ func createSymlink(sourceSkillDir, agentSkillsDir, linkPath string) error { return copyDir(sourceSkillDir, linkPath) } -// copyDir recursively copies src into a newly created dst directory. +// copyDir replaces dst with an exact copy of src. +// Any files in dst that no longer exist in src are removed, so the installed copy +// stays in sync with the canonical source on skill updates. func copyDir(src, dst string) error { - if err := os.MkdirAll(dst, 0o755); err != nil { - return fmt.Errorf("create copy dir: %w", err) + tmpDst, err := os.MkdirTemp(filepath.Dir(dst), ".skill-copy-*") + if err != nil { + return fmt.Errorf("create temp copy dir: %w", err) + } + if err := mergeDir(src, tmpDst); err != nil { + _ = os.RemoveAll(tmpDst) + return err + } + if err := os.RemoveAll(dst); err != nil { + _ = os.RemoveAll(tmpDst) + return fmt.Errorf("remove stale copy dir: %w", err) } - return mergeDir(src, dst) + if err := os.Rename(tmpDst, dst); err != nil { + // Cross-filesystem fallback: re-create dst from the temp copy. + if mkErr := os.MkdirAll(dst, 0o755); mkErr != nil { + _ = os.RemoveAll(tmpDst) + return fmt.Errorf("create copy dir: %w", mkErr) + } + if mergeErr := mergeDir(tmpDst, dst); mergeErr != nil { + _ = os.RemoveAll(tmpDst) + return mergeErr + } + _ = os.RemoveAll(tmpDst) + } + return nil } // RemoveSkillLink removes the skill entry (symlink or copied directory) at agentSkillsDir/skillName. @@ -111,7 +141,10 @@ func CheckSkillLink(agentSkillsDir, skillName, expectedSourceDir string) string linkPath := filepath.Join(agentSkillsDir, skillName) info, err := os.Lstat(linkPath) if err != nil { - return "missing" + if os.IsNotExist(err) { + return "missing" + } + return "broken" } if info.Mode()&os.ModeSymlink == 0 { diff --git a/internal/ai/skills/symlink_test.go b/internal/ai/skills/symlink_test.go index 15c80b640..1a84a245b 100644 --- a/internal/ai/skills/symlink_test.go +++ b/internal/ai/skills/symlink_test.go @@ -74,6 +74,20 @@ func TestCheckSkillLink(t *testing.T) { assert.Equal(t, "copy", CheckSkillLink(agentDir, "my-skill", "/any/source")) }) + + t.Run("broken on permission error (not missing)", func(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("root bypasses permission checks") + } + parent := t.TempDir() + agentDir := filepath.Join(parent, "locked") + require.NoError(t, os.MkdirAll(filepath.Join(agentDir, "my-skill"), 0o755)) + require.NoError(t, os.Chmod(agentDir, 0o000)) + t.Cleanup(func() { _ = os.Chmod(agentDir, 0o755) }) + + result := CheckSkillLink(agentDir, "my-skill", "/any/source") + assert.Equal(t, "broken", result) + }) } // --- CreateSkillLink --- @@ -168,18 +182,51 @@ func TestCreateSkillLink(t *testing.T) { assert.Equal(t, "copy", CheckSkillLink(agentDir, "my-skill", src)) }) - t.Run("skips real directory when useCopy is false", func(t *testing.T) { + t.Run("warns and skips real directory when useCopy is false", func(t *testing.T) { agentDir := t.TempDir() linkPath := filepath.Join(agentDir, "my-skill") require.NoError(t, os.MkdirAll(linkPath, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(linkPath, "SKILL.md"), []byte("original"), 0o644)) src := makeSkillSource(t) + // Should succeed (skip) but warn to stderr. require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", false)) + // Original directory content must be preserved. data, err := os.ReadFile(filepath.Join(linkPath, "SKILL.md")) require.NoError(t, err) assert.Equal(t, "original", string(data), "original directory should be preserved") + // Entry must still be a real directory, not a symlink. + info, err := os.Lstat(linkPath) + require.NoError(t, err) + assert.Zero(t, info.Mode()&os.ModeSymlink, "entry should remain a directory") + }) + + t.Run("errors on regular file at linkPath", func(t *testing.T) { + agentDir := t.TempDir() + linkPath := filepath.Join(agentDir, "my-skill") + require.NoError(t, os.WriteFile(linkPath, []byte("not a dir"), 0o644)) + + src := makeSkillSource(t) + err := CreateSkillLink(src, agentDir, "my-skill", false) + assert.Error(t, err) + }) + + t.Run("copy is replaced on re-install (replace semantics)", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", true)) + + // Add a stale file directly into the copy. + staleFile := filepath.Join(agentDir, "my-skill", "stale.txt") + require.NoError(t, os.WriteFile(staleFile, []byte("stale"), 0o644)) + + // Re-run copy install; the stale file should be gone. + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", true)) + + _, err := os.Stat(staleFile) + assert.True(t, os.IsNotExist(err), "stale file should be removed after re-install") }) } From 37cf248e8ce9773fb49aff6d5409e962eabb28f1 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Mon, 8 Jun 2026 00:17:48 +0530 Subject: [PATCH 21/23] fix: use os.samefile for symlink check, remove tmo file in copy dir --- internal/ai/skills/fs_util.go | 3 ++ internal/ai/skills/fs_util_test.go | 76 ++++++++++++++++++++++++++++++ internal/ai/skills/symlink.go | 60 +++++++++++++---------- internal/ai/skills/symlink_test.go | 38 +++++++++++++++ 4 files changed, 151 insertions(+), 26 deletions(-) create mode 100644 internal/ai/skills/fs_util_test.go diff --git a/internal/ai/skills/fs_util.go b/internal/ai/skills/fs_util.go index 025d13d2c..e60e84584 100644 --- a/internal/ai/skills/fs_util.go +++ b/internal/ai/skills/fs_util.go @@ -22,6 +22,9 @@ func mergeDir(src, dst string) error { if err != nil { return err } + // os.Symlink is not idempotent (returns EEXIST). Remove any existing + // entry so the call is safe under concurrent writes or repeated merges. + _ = os.Remove(dstPath) if err := os.Symlink(target, dstPath); err != nil { return err } diff --git a/internal/ai/skills/fs_util_test.go b/internal/ai/skills/fs_util_test.go new file mode 100644 index 000000000..eabd86582 --- /dev/null +++ b/internal/ai/skills/fs_util_test.go @@ -0,0 +1,76 @@ +package skills + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMergeDir(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink tests skipped on windows") + } + + t.Run("copies regular files", func(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(src, "file.txt"), []byte("hello"), 0o644)) + + require.NoError(t, mergeDir(src, dst)) + + data, err := os.ReadFile(filepath.Join(dst, "file.txt")) + require.NoError(t, err) + assert.Equal(t, "hello", string(data)) + }) + + t.Run("preserves symlinks", func(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + target := filepath.Join(src, "target.txt") + require.NoError(t, os.WriteFile(target, []byte("target"), 0o644)) + require.NoError(t, os.Symlink(target, filepath.Join(src, "link"))) + + require.NoError(t, mergeDir(src, dst)) + + linkDst := filepath.Join(dst, "link") + info, err := os.Lstat(linkDst) + require.NoError(t, err) + assert.NotZero(t, info.Mode()&os.ModeSymlink, "should be a symlink") + }) + + t.Run("symlink overwrite is idempotent (no EEXIST)", func(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + target := filepath.Join(src, "target.txt") + require.NoError(t, os.WriteFile(target, []byte("target"), 0o644)) + require.NoError(t, os.Symlink(target, filepath.Join(src, "link"))) + + // First merge creates the symlink. + require.NoError(t, mergeDir(src, dst)) + // Second merge must not fail with EEXIST. + require.NoError(t, mergeDir(src, dst)) + + linkDst := filepath.Join(dst, "link") + info, err := os.Lstat(linkDst) + require.NoError(t, err) + assert.NotZero(t, info.Mode()&os.ModeSymlink) + }) + + t.Run("recurses into subdirectories", func(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + sub := filepath.Join(src, "sub") + require.NoError(t, os.MkdirAll(sub, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(sub, "nested.txt"), []byte("nested"), 0o644)) + + require.NoError(t, mergeDir(src, dst)) + + data, err := os.ReadFile(filepath.Join(dst, "sub", "nested.txt")) + require.NoError(t, err) + assert.Equal(t, "nested", string(data)) + }) +} diff --git a/internal/ai/skills/symlink.go b/internal/ai/skills/symlink.go index 8f741e4ab..3da05ab73 100644 --- a/internal/ai/skills/symlink.go +++ b/internal/ai/skills/symlink.go @@ -2,12 +2,16 @@ package skills import ( "fmt" + "io" "os" "os/exec" "path/filepath" "runtime" ) +// stderrWriter is the target for diagnostic output. Replaced in tests. +var stderrWriter io.Writer = os.Stderr + // CreateSkillLink installs skillName from sourceSkillDir into agentSkillsDir. // useCopy=true copies files recursively; useCopy=false creates a symlink. // The operation is idempotent: a correct existing symlink or copy is left unchanged. @@ -21,7 +25,9 @@ func CreateSkillLink(sourceSkillDir, agentSkillsDir, skillName string, useCopy b info, err := os.Lstat(linkPath) if err == nil { if info.Mode()&os.ModeSymlink != 0 { - if isSymlinkCorrect(linkPath, agentSkillsDir, sourceSkillDir) { + // For useCopy=false: skip if already pointing to the right place. + // For useCopy=true: remove the symlink so we can replace it with a copy. + if !useCopy && isSymlinkCorrect(linkPath, sourceSkillDir) { return nil } if rmErr := os.Remove(linkPath); rmErr != nil { @@ -29,7 +35,7 @@ func CreateSkillLink(sourceSkillDir, agentSkillsDir, skillName string, useCopy b } } else if info.IsDir() { if !useCopy { - fmt.Fprintf(os.Stderr, + fmt.Fprintf(stderrWriter, "warning: %s is a copied directory; remove it manually to switch to symlink mode\n", linkPath) return nil @@ -49,19 +55,17 @@ func CreateSkillLink(sourceSkillDir, agentSkillsDir, skillName string, useCopy b } // isSymlinkCorrect returns true if linkPath is a non-broken symlink resolving to sourceSkillDir. -func isSymlinkCorrect(linkPath, agentSkillsDir, sourceSkillDir string) bool { - target, err := os.Readlink(linkPath) +// os.SameFile is used instead of string comparison to handle case-insensitive filesystems (e.g. macOS APFS). +func isSymlinkCorrect(linkPath, sourceSkillDir string) bool { + linkInfo, err := os.Stat(linkPath) if err != nil { - return false - } - if !filepath.IsAbs(target) { - target = filepath.Join(agentSkillsDir, target) + return false // broken symlink } - if filepath.Clean(target) != filepath.Clean(sourceSkillDir) { + srcInfo, err := os.Stat(sourceSkillDir) + if err != nil { return false } - _, err = os.Stat(linkPath) - return err == nil + return os.SameFile(linkInfo, srcInfo) } // createSymlink creates a symlink at linkPath pointing to sourceSkillDir. @@ -83,7 +87,7 @@ func createSymlink(sourceSkillDir, agentSkillsDir, linkPath string) error { if err := exec.Command("cmd", "/C", "mklink", "/J", linkPath, sourceSkillDir).Run(); err == nil { return nil } - fmt.Fprintf(os.Stderr, "warning: symlink and junction unavailable; copying %s to %s\n", sourceSkillDir, linkPath) + fmt.Fprintf(stderrWriter, "warning: symlink and junction unavailable; copying %s to %s\n", sourceSkillDir, linkPath) return copyDir(sourceSkillDir, linkPath) } @@ -95,25 +99,31 @@ func copyDir(src, dst string) error { if err != nil { return fmt.Errorf("create temp copy dir: %w", err) } + // Always clean up the temp dir so it is never left as an orphan in agentSkillsDir. + tmpRemoved := false + defer func() { + if !tmpRemoved { + _ = os.RemoveAll(tmpDst) + } + }() + if err := mergeDir(src, tmpDst); err != nil { - _ = os.RemoveAll(tmpDst) return err } if err := os.RemoveAll(dst); err != nil { - _ = os.RemoveAll(tmpDst) return fmt.Errorf("remove stale copy dir: %w", err) } if err := os.Rename(tmpDst, dst); err != nil { // Cross-filesystem fallback: re-create dst from the temp copy. + fmt.Fprintf(stderrWriter, "warning: rename %s → %s failed (%v); falling back to copy\n", tmpDst, dst, err) if mkErr := os.MkdirAll(dst, 0o755); mkErr != nil { - _ = os.RemoveAll(tmpDst) return fmt.Errorf("create copy dir: %w", mkErr) } if mergeErr := mergeDir(tmpDst, dst); mergeErr != nil { - _ = os.RemoveAll(tmpDst) return mergeErr } - _ = os.RemoveAll(tmpDst) + } else { + tmpRemoved = true // rename succeeded; temp dir is now dst } return nil } @@ -151,20 +161,18 @@ func CheckSkillLink(agentSkillsDir, skillName, expectedSourceDir string) string return "copy" } - // It's a symlink. Verify the target exists. - if _, err := os.Stat(linkPath); err != nil { + // It's a symlink. Verify the target exists by following the link. + resolvedInfo, err := os.Stat(linkPath) + if err != nil { return "broken" } - // Target exists. Check if it points to the expected place. - target, err := os.Readlink(linkPath) + // Use os.SameFile to handle case-insensitive filesystems (e.g. macOS APFS). + srcInfo, err := os.Stat(expectedSourceDir) if err != nil { - return "broken" - } - if !filepath.IsAbs(target) { - target = filepath.Join(agentSkillsDir, target) + return "wrong_target" } - if filepath.Clean(target) == filepath.Clean(expectedSourceDir) { + if os.SameFile(resolvedInfo, srcInfo) { return "ok" } return "wrong_target" diff --git a/internal/ai/skills/symlink_test.go b/internal/ai/skills/symlink_test.go index 1a84a245b..969565896 100644 --- a/internal/ai/skills/symlink_test.go +++ b/internal/ai/skills/symlink_test.go @@ -1,15 +1,27 @@ package skills import ( + "bytes" "os" "path/filepath" "runtime" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// captureStderr replaces stderrWriter with a buffer for the duration of the test. +func captureStderr(t *testing.T) *bytes.Buffer { + t.Helper() + buf := &bytes.Buffer{} + orig := stderrWriter + stderrWriter = buf + t.Cleanup(func() { stderrWriter = orig }) + return buf +} + // makeSkillSource creates a temporary directory with a SKILL.md file inside. func makeSkillSource(t *testing.T) string { t.Helper() @@ -183,6 +195,7 @@ func TestCreateSkillLink(t *testing.T) { }) t.Run("warns and skips real directory when useCopy is false", func(t *testing.T) { + buf := captureStderr(t) agentDir := t.TempDir() linkPath := filepath.Join(agentDir, "my-skill") require.NoError(t, os.MkdirAll(linkPath, 0o755)) @@ -200,6 +213,8 @@ func TestCreateSkillLink(t *testing.T) { info, err := os.Lstat(linkPath) require.NoError(t, err) assert.Zero(t, info.Mode()&os.ModeSymlink, "entry should remain a directory") + // Warning must be emitted to stderr. + assert.True(t, strings.Contains(buf.String(), "warning:"), "expected warning on stderr, got: %q", buf.String()) }) t.Run("errors on regular file at linkPath", func(t *testing.T) { @@ -228,6 +243,29 @@ func TestCreateSkillLink(t *testing.T) { _, err := os.Stat(staleFile) assert.True(t, os.IsNotExist(err), "stale file should be removed after re-install") }) + + t.Run("converts existing correct symlink to copy when useCopy switches to true", func(t *testing.T) { + src := makeSkillSource(t) + agentDir := t.TempDir() + + // Install as symlink first. + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", false)) + assert.Equal(t, "ok", CheckSkillLink(agentDir, "my-skill", src)) + info, err := os.Lstat(filepath.Join(agentDir, "my-skill")) + require.NoError(t, err) + assert.NotZero(t, info.Mode()&os.ModeSymlink, "should be a symlink after first install") + + // Re-install with useCopy=true; the symlink must be replaced by a real directory. + require.NoError(t, CreateSkillLink(src, agentDir, "my-skill", true)) + assert.Equal(t, "copy", CheckSkillLink(agentDir, "my-skill", src)) + info, err = os.Lstat(filepath.Join(agentDir, "my-skill")) + require.NoError(t, err) + assert.Zero(t, info.Mode()&os.ModeSymlink, "should be a real directory after copy install") + data, err := os.ReadFile(filepath.Join(agentDir, "my-skill", "SKILL.md")) + require.NoError(t, err) + assert.Equal(t, "# skill", string(data)) + }) + } // --- RemoveSkillLink --- From eedee4d6201d348dbe6d5d2c1a46ad2475ada231 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Mon, 8 Jun 2026 01:01:56 +0530 Subject: [PATCH 22/23] feat: validate skills installation --- internal/ai/skills/validate.go | 82 +++++++ internal/ai/skills/validate_test.go | 348 ++++++++++++++++++++++++++++ 2 files changed, 430 insertions(+) create mode 100644 internal/ai/skills/validate.go create mode 100644 internal/ai/skills/validate_test.go diff --git a/internal/ai/skills/validate.go b/internal/ai/skills/validate.go new file mode 100644 index 000000000..f2e057568 --- /dev/null +++ b/internal/ai/skills/validate.go @@ -0,0 +1,82 @@ +package skills + +import ( + "fmt" + "os" + "path/filepath" +) + +// SkillInstallStatus reports the installation state of one (agent, skill) pair. +type SkillInstallStatus struct { + SkillName string + AgentID string + LinkPath string + Status string // "ok" | "missing" | "broken_symlink" | "invalid_skill" | "copy" | "unknown" + Error string +} + +// ValidateInstall checks each skill in skills for the given agent. +// sourcePluginDir is the directory containing skill subdirectories (pluginDir/skills/). +func ValidateInstall(agentID, agentSkillsDir, sourcePluginDir string, skills []string) []SkillInstallStatus { + out := make([]SkillInstallStatus, 0, len(skills)) + for _, skillName := range skills { + expectedSource := filepath.Join(sourcePluginDir, skillName) + linkPath := filepath.Join(agentSkillsDir, skillName) + + s := SkillInstallStatus{ + SkillName: skillName, + AgentID: agentID, + LinkPath: linkPath, + } + + switch CheckSkillLink(agentSkillsDir, skillName, expectedSource) { + case "missing": + s.Status = "missing" + case "broken": + s.Status = "broken_symlink" + s.Error = "symlink target does not exist or is inaccessible" + case "wrong_target": + s.Status = "broken_symlink" + s.Error = "symlink points to wrong target" + case "ok": + if err := checkSkillMeta(expectedSource, skillName); err != nil { + s.Status = "invalid_skill" + s.Error = err.Error() + } else { + s.Status = "ok" + } + case "copy": + fi, statErr := os.Stat(linkPath) + if statErr != nil || !fi.IsDir() { + s.Status = "invalid_skill" + s.Error = fmt.Sprintf("%s is a regular file, not a skill directory", linkPath) + } else if err := checkSkillMeta(linkPath, skillName); err != nil { + s.Status = "invalid_skill" + s.Error = err.Error() + } else { + s.Status = "copy" + } + default: + s.Status = "unknown" + s.Error = "unexpected link state" + } + + out = append(out, s) + } + return out +} + +// checkSkillMeta verifies that skillDir contains a readable SKILL.md whose name field matches skillName. +func checkSkillMeta(skillDir, skillName string) error { + meta, err := ParseSkillMeta(skillDir) + if err != nil { + return fmt.Errorf("read SKILL.md: %w", err) + } + if meta.Name == "" { + return fmt.Errorf("SKILL.md has no frontmatter or empty name field") + } + if meta.Name != skillName { + return fmt.Errorf("SKILL.md name %q does not match directory name %q", meta.Name, skillName) + } + return nil +} diff --git a/internal/ai/skills/validate_test.go b/internal/ai/skills/validate_test.go new file mode 100644 index 000000000..abfb99686 --- /dev/null +++ b/internal/ai/skills/validate_test.go @@ -0,0 +1,348 @@ +package skills + +import ( + "os" + "path/filepath" + "testing" +) + +// makeSkillDir creates skillDir/SKILL.md with a frontmatter block. +func makeSkillDir(t *testing.T, base, skillName string) string { + t.Helper() + dir := filepath.Join(base, skillName) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + content := "---\nname: " + skillName + "\ndescription: test skill\n---\n# body\n" + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + return dir +} + +func TestValidateInstall_OK(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + + makeSkillDir(t, sourcePluginDir, "auth0-react") + + if err := CreateSkillLink( + filepath.Join(sourcePluginDir, "auth0-react"), + agentSkillsDir, "auth0-react", false, + ); err != nil { + t.Fatalf("CreateSkillLink: %v", err) + } + + statuses := ValidateInstall("claude-code", agentSkillsDir, sourcePluginDir, []string{"auth0-react"}) + if len(statuses) != 1 { + t.Fatalf("expected 1 status, got %d", len(statuses)) + } + s := statuses[0] + if s.Status != "ok" { + t.Errorf("expected ok, got %q (err: %s)", s.Status, s.Error) + } + if s.SkillName != "auth0-react" { + t.Errorf("unexpected SkillName: %q", s.SkillName) + } + if s.AgentID != "claude-code" { + t.Errorf("unexpected AgentID: %q", s.AgentID) + } +} + +func TestValidateInstall_Missing(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + if err := os.MkdirAll(agentSkillsDir, 0o755); err != nil { + t.Fatal(err) + } + + statuses := ValidateInstall("cursor", agentSkillsDir, sourcePluginDir, []string{"auth0-nextjs"}) + if len(statuses) != 1 { + t.Fatalf("expected 1, got %d", len(statuses)) + } + if statuses[0].Status != "missing" { + t.Errorf("expected missing, got %q", statuses[0].Status) + } +} + +func TestValidateInstall_BrokenSymlink(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + if err := os.MkdirAll(agentSkillsDir, 0o755); err != nil { + t.Fatal(err) + } + + // Create a symlink pointing to a non-existent path inside tmp (portable). + linkPath := filepath.Join(agentSkillsDir, "auth0-vue") + if err := os.Symlink(filepath.Join(tmp, "nonexistent", "auth0-vue"), linkPath); err != nil { + t.Fatal(err) + } + + statuses := ValidateInstall("gemini-cli", agentSkillsDir, sourcePluginDir, []string{"auth0-vue"}) + if statuses[0].Status != "broken_symlink" { + t.Errorf("expected broken_symlink, got %q", statuses[0].Status) + } + if statuses[0].Error == "" { + t.Error("expected non-empty Error for broken_symlink") + } +} + +func TestValidateInstall_WrongTarget(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + wrongSourceDir := filepath.Join(tmp, "other") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + + makeSkillDir(t, sourcePluginDir, "auth0-react") + makeSkillDir(t, wrongSourceDir, "auth0-react") + + // Link points to wrong source. + if err := CreateSkillLink( + filepath.Join(wrongSourceDir, "auth0-react"), + agentSkillsDir, "auth0-react", false, + ); err != nil { + t.Fatalf("CreateSkillLink: %v", err) + } + + statuses := ValidateInstall("cursor", agentSkillsDir, sourcePluginDir, []string{"auth0-react"}) + if statuses[0].Status != "broken_symlink" { + t.Errorf("expected broken_symlink, got %q", statuses[0].Status) + } + if statuses[0].Error == "" { + t.Error("expected non-empty Error for wrong_target") + } +} + +func TestValidateInstall_InvalidSkill_MissingFile(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + + // Create source dir without SKILL.md. + skillSrc := filepath.Join(sourcePluginDir, "auth0-spa") + if err := os.MkdirAll(skillSrc, 0o755); err != nil { + t.Fatal(err) + } + + if err := CreateSkillLink(skillSrc, agentSkillsDir, "auth0-spa", false); err != nil { + t.Fatalf("CreateSkillLink: %v", err) + } + + statuses := ValidateInstall("claude-code", agentSkillsDir, sourcePluginDir, []string{"auth0-spa"}) + if statuses[0].Status != "invalid_skill" { + t.Errorf("expected invalid_skill, got %q", statuses[0].Status) + } + if statuses[0].Error == "" { + t.Error("expected non-empty Error field") + } +} + +func TestValidateInstall_InvalidSkill_NameMismatch(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + + // Create skill dir with mismatched name in frontmatter. + skillSrc := filepath.Join(sourcePluginDir, "auth0-angular") + if err := os.MkdirAll(skillSrc, 0o755); err != nil { + t.Fatal(err) + } + content := "---\nname: totally-different\ndescription: x\n---\n" + if err := os.WriteFile(filepath.Join(skillSrc, "SKILL.md"), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + + if err := CreateSkillLink(skillSrc, agentSkillsDir, "auth0-angular", false); err != nil { + t.Fatalf("CreateSkillLink: %v", err) + } + + statuses := ValidateInstall("claude-code", agentSkillsDir, sourcePluginDir, []string{"auth0-angular"}) + if statuses[0].Status != "invalid_skill" { + t.Errorf("expected invalid_skill, got %q", statuses[0].Status) + } +} + +func TestValidateInstall_CopyMode(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + + makeSkillDir(t, sourcePluginDir, "auth0-nextjs") + + if err := CreateSkillLink( + filepath.Join(sourcePluginDir, "auth0-nextjs"), + agentSkillsDir, "auth0-nextjs", true, + ); err != nil { + t.Fatalf("CreateSkillLink (copy): %v", err) + } + + statuses := ValidateInstall("cursor", agentSkillsDir, sourcePluginDir, []string{"auth0-nextjs"}) + if statuses[0].Status != "copy" { + t.Errorf("expected copy, got %q", statuses[0].Status) + } +} + +func TestValidateInstall_MultipleSkills(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + + makeSkillDir(t, sourcePluginDir, "skill-a") + makeSkillDir(t, sourcePluginDir, "skill-b") + + if err := CreateSkillLink(filepath.Join(sourcePluginDir, "skill-a"), agentSkillsDir, "skill-a", false); err != nil { + t.Fatal(err) + } + // skill-b intentionally not installed. + + statuses := ValidateInstall("claude-code", agentSkillsDir, sourcePluginDir, []string{"skill-a", "skill-b"}) + if len(statuses) != 2 { + t.Fatalf("expected 2 statuses, got %d", len(statuses)) + } + statusMap := map[string]string{} + for _, s := range statuses { + statusMap[s.SkillName] = s.Status + } + if statusMap["skill-a"] != "ok" { + t.Errorf("skill-a: expected ok, got %q", statusMap["skill-a"]) + } + if statusMap["skill-b"] != "missing" { + t.Errorf("skill-b: expected missing, got %q", statusMap["skill-b"]) + } +} + +func TestValidateInstall_LinkPathField(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + if err := os.MkdirAll(agentSkillsDir, 0o755); err != nil { + t.Fatal(err) + } + + statuses := ValidateInstall("claude-code", agentSkillsDir, sourcePluginDir, []string{"auth0-react"}) + expected := filepath.Join(agentSkillsDir, "auth0-react") + if statuses[0].LinkPath != expected { + t.Errorf("expected LinkPath %q, got %q", expected, statuses[0].LinkPath) + } +} + +func TestValidateInstall_EmptySkillsList(t *testing.T) { + tmp := t.TempDir() + statuses := ValidateInstall("claude-code", tmp, tmp, []string{}) + if len(statuses) != 0 { + t.Errorf("expected empty slice, got %d entries", len(statuses)) + } +} + +func TestValidateInstall_AbsentAgentSkillsDir(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + // agentSkillsDir does not exist — all requested skills should return "missing". + agentSkillsDir := filepath.Join(tmp, "nonexistent", "agent", "skills") + + statuses := ValidateInstall("claude-code", agentSkillsDir, sourcePluginDir, []string{"auth0-react", "auth0-nextjs"}) + if len(statuses) != 2 { + t.Fatalf("expected 2 statuses, got %d", len(statuses)) + } + for _, s := range statuses { + if s.Status != "missing" { + t.Errorf("skill %q: expected missing when agentSkillsDir absent, got %q", s.SkillName, s.Status) + } + } +} + +func TestValidateInstall_CopyMode_InvalidSkillMd(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + + // Create source dir with a SKILL.md that has no frontmatter. + skillSrc := filepath.Join(sourcePluginDir, "auth0-spa") + if err := os.MkdirAll(skillSrc, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillSrc, "SKILL.md"), []byte("no frontmatter here"), 0o644); err != nil { + t.Fatal(err) + } + + if err := CreateSkillLink(skillSrc, agentSkillsDir, "auth0-spa", true); err != nil { + t.Fatalf("CreateSkillLink (copy): %v", err) + } + + statuses := ValidateInstall("cursor", agentSkillsDir, sourcePluginDir, []string{"auth0-spa"}) + if statuses[0].Status != "invalid_skill" { + t.Errorf("expected invalid_skill for copy with no frontmatter, got %q", statuses[0].Status) + } + if statuses[0].Error == "" { + t.Error("expected non-empty Error field") + } +} + +func TestValidateInstall_RegularFileAtLinkPath(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + if err := os.MkdirAll(agentSkillsDir, 0o755); err != nil { + t.Fatal(err) + } + + // Place a regular file where a skill directory should be. + linkPath := filepath.Join(agentSkillsDir, "auth0-react") + if err := os.WriteFile(linkPath, []byte("not a directory"), 0o644); err != nil { + t.Fatal(err) + } + + statuses := ValidateInstall("claude-code", agentSkillsDir, sourcePluginDir, []string{"auth0-react"}) + if statuses[0].Status != "invalid_skill" { + t.Errorf("expected invalid_skill for regular file at linkPath, got %q", statuses[0].Status) + } + if statuses[0].Error == "" { + t.Error("expected non-empty Error field") + } +} + +func TestValidateInstall_NoFrontmatter_ClearError(t *testing.T) { + tmp := t.TempDir() + sourcePluginDir := filepath.Join(tmp, "plugins") + agentSkillsDir := filepath.Join(tmp, "agent", "skills") + + // Skill dir has SKILL.md but with no --- delimiters. + skillSrc := filepath.Join(sourcePluginDir, "auth0-vue") + if err := os.MkdirAll(skillSrc, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillSrc, "SKILL.md"), []byte("# Just a heading, no frontmatter\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := CreateSkillLink(skillSrc, agentSkillsDir, "auth0-vue", false); err != nil { + t.Fatalf("CreateSkillLink: %v", err) + } + + statuses := ValidateInstall("claude-code", agentSkillsDir, sourcePluginDir, []string{"auth0-vue"}) + if statuses[0].Status != "invalid_skill" { + t.Errorf("expected invalid_skill, got %q", statuses[0].Status) + } + // Error should mention missing frontmatter, not a name mismatch. + if statuses[0].Error == "" { + t.Error("expected non-empty Error") + } + const wantSubstr = "no frontmatter" + if !contains(statuses[0].Error, wantSubstr) { + t.Errorf("expected error to contain %q, got %q", wantSubstr, statuses[0].Error) + } +} + +func contains(s, sub string) bool { + return len(s) >= len(sub) && (s == sub || len(sub) == 0 || + func() bool { + for i := 0; i <= len(s)-len(sub); i++ { + if s[i:i+len(sub)] == sub { + return true + } + } + return false + }()) +} From f2943b104c2fc31a5972587ed47de76018bc0676 Mon Sep 17 00:00:00 2001 From: Kartik Jha Date: Mon, 8 Jun 2026 02:02:58 +0530 Subject: [PATCH 23/23] fix: lint fixes --- internal/ai/skills/fs_util.go | 2 +- internal/ai/skills/symlink.go | 17 +++++++++-------- internal/ai/skills/symlink_test.go | 7 +++---- internal/ai/skills/validate.go | 6 +++--- internal/ai/skills/validate_test.go | 4 ++-- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/internal/ai/skills/fs_util.go b/internal/ai/skills/fs_util.go index e60e84584..c1195cc2f 100644 --- a/internal/ai/skills/fs_util.go +++ b/internal/ai/skills/fs_util.go @@ -22,7 +22,7 @@ func mergeDir(src, dst string) error { if err != nil { return err } - // os.Symlink is not idempotent (returns EEXIST). Remove any existing + // Os.Symlink is not idempotent (returns EEXIST). Remove any existing // entry so the call is safe under concurrent writes or repeated merges. _ = os.Remove(dstPath) if err := os.Symlink(target, dstPath); err != nil { diff --git a/internal/ai/skills/symlink.go b/internal/ai/skills/symlink.go index 3da05ab73..0a0f5af3d 100644 --- a/internal/ai/skills/symlink.go +++ b/internal/ai/skills/symlink.go @@ -13,7 +13,7 @@ import ( var stderrWriter io.Writer = os.Stderr // CreateSkillLink installs skillName from sourceSkillDir into agentSkillsDir. -// useCopy=true copies files recursively; useCopy=false creates a symlink. +// When useCopy is true the directory is copied recursively; otherwise a symlink is created. // The operation is idempotent: a correct existing symlink or copy is left unchanged. func CreateSkillLink(sourceSkillDir, agentSkillsDir, skillName string, useCopy bool) error { if err := os.MkdirAll(agentSkillsDir, 0o755); err != nil { @@ -24,7 +24,8 @@ func CreateSkillLink(sourceSkillDir, agentSkillsDir, skillName string, useCopy b info, err := os.Lstat(linkPath) if err == nil { - if info.Mode()&os.ModeSymlink != 0 { + switch { + case info.Mode()&os.ModeSymlink != 0: // For useCopy=false: skip if already pointing to the right place. // For useCopy=true: remove the symlink so we can replace it with a copy. if !useCopy && isSymlinkCorrect(linkPath, sourceSkillDir) { @@ -33,15 +34,15 @@ func CreateSkillLink(sourceSkillDir, agentSkillsDir, skillName string, useCopy b if rmErr := os.Remove(linkPath); rmErr != nil { return fmt.Errorf("remove existing symlink %s: %w", linkPath, rmErr) } - } else if info.IsDir() { + case info.IsDir(): if !useCopy { fmt.Fprintf(stderrWriter, "warning: %s is a copied directory; remove it manually to switch to symlink mode\n", linkPath) return nil } - // useCopy=true: fall through to re-copy with replace semantics. - } else { + // UseCopy=true: fall through to re-copy with replace semantics. + default: return fmt.Errorf("%s exists as a regular file; remove it before installing skill %q", linkPath, skillName) } } else if !os.IsNotExist(err) { @@ -55,11 +56,11 @@ func CreateSkillLink(sourceSkillDir, agentSkillsDir, skillName string, useCopy b } // isSymlinkCorrect returns true if linkPath is a non-broken symlink resolving to sourceSkillDir. -// os.SameFile is used instead of string comparison to handle case-insensitive filesystems (e.g. macOS APFS). +// Uses os.SameFile instead of string comparison to handle case-insensitive filesystems (e.g. macOS APFS). func isSymlinkCorrect(linkPath, sourceSkillDir string) bool { linkInfo, err := os.Stat(linkPath) if err != nil { - return false // broken symlink + return false // Broken symlink. } srcInfo, err := os.Stat(sourceSkillDir) if err != nil { @@ -123,7 +124,7 @@ func copyDir(src, dst string) error { return mergeErr } } else { - tmpRemoved = true // rename succeeded; temp dir is now dst + tmpRemoved = true // Rename succeeded; temp dir is now dst. } return nil } diff --git a/internal/ai/skills/symlink_test.go b/internal/ai/skills/symlink_test.go index 969565896..3f9ff3f36 100644 --- a/internal/ai/skills/symlink_test.go +++ b/internal/ai/skills/symlink_test.go @@ -30,7 +30,7 @@ func makeSkillSource(t *testing.T) string { return dir } -// --- CheckSkillLink --- +// --- CheckSkillLink ---. func TestCheckSkillLink(t *testing.T) { if runtime.GOOS == "windows" { @@ -102,7 +102,7 @@ func TestCheckSkillLink(t *testing.T) { }) } -// --- CreateSkillLink --- +// --- CreateSkillLink ---. func TestCreateSkillLink(t *testing.T) { if runtime.GOOS == "windows" { @@ -265,10 +265,9 @@ func TestCreateSkillLink(t *testing.T) { require.NoError(t, err) assert.Equal(t, "# skill", string(data)) }) - } -// --- RemoveSkillLink --- +// --- RemoveSkillLink ---. func TestRemoveSkillLink(t *testing.T) { if runtime.GOOS == "windows" { diff --git a/internal/ai/skills/validate.go b/internal/ai/skills/validate.go index f2e057568..846fd6fef 100644 --- a/internal/ai/skills/validate.go +++ b/internal/ai/skills/validate.go @@ -11,12 +11,12 @@ type SkillInstallStatus struct { SkillName string AgentID string LinkPath string - Status string // "ok" | "missing" | "broken_symlink" | "invalid_skill" | "copy" | "unknown" + Status string // "ok" | "missing" | "broken_symlink" | "invalid_skill" | "copy" | "unknown". Error string } -// ValidateInstall checks each skill in skills for the given agent. -// sourcePluginDir is the directory containing skill subdirectories (pluginDir/skills/). +// ValidateInstall checks the installation state of each skill in agentSkillsDir. +// The sourcePluginDir parameter is the directory containing skill subdirectories (pluginDir/skills/). func ValidateInstall(agentID, agentSkillsDir, sourcePluginDir string, skills []string) []SkillInstallStatus { out := make([]SkillInstallStatus, 0, len(skills)) for _, skillName := range skills { diff --git a/internal/ai/skills/validate_test.go b/internal/ai/skills/validate_test.go index abfb99686..6876bf5fb 100644 --- a/internal/ai/skills/validate_test.go +++ b/internal/ai/skills/validate_test.go @@ -196,7 +196,7 @@ func TestValidateInstall_MultipleSkills(t *testing.T) { if err := CreateSkillLink(filepath.Join(sourcePluginDir, "skill-a"), agentSkillsDir, "skill-a", false); err != nil { t.Fatal(err) } - // skill-b intentionally not installed. + // Skill-b intentionally not installed. statuses := ValidateInstall("claude-code", agentSkillsDir, sourcePluginDir, []string{"skill-a", "skill-b"}) if len(statuses) != 2 { @@ -240,7 +240,7 @@ func TestValidateInstall_EmptySkillsList(t *testing.T) { func TestValidateInstall_AbsentAgentSkillsDir(t *testing.T) { tmp := t.TempDir() sourcePluginDir := filepath.Join(tmp, "plugins") - // agentSkillsDir does not exist — all requested skills should return "missing". + // AgentSkillsDir does not exist — all requested skills should return "missing". agentSkillsDir := filepath.Join(tmp, "nonexistent", "agent", "skills") statuses := ValidateInstall("claude-code", agentSkillsDir, sourcePluginDir, []string{"auth0-react", "auth0-nextjs"})