From 64fd9d13f4e05a5253c562e885c1e0709c24ac3e Mon Sep 17 00:00:00 2001 From: Arseny Kravchenko Date: Tue, 17 Feb 2026 16:01:42 +0100 Subject: [PATCH 1/3] Recommend skills installation after apps init --- cmd/apps/init.go | 6 + experimental/aitools/lib/agents/recommend.go | 48 +++++++ .../aitools/lib/agents/recommend_test.go | 92 +++++++++++++ experimental/aitools/lib/agents/skills.go | 58 ++++++++ .../aitools/lib/agents/skills_test.go | 129 ++++++++++++++++++ 5 files changed, 333 insertions(+) create mode 100644 experimental/aitools/lib/agents/recommend.go create mode 100644 experimental/aitools/lib/agents/recommend_test.go create mode 100644 experimental/aitools/lib/agents/skills.go create mode 100644 experimental/aitools/lib/agents/skills_test.go diff --git a/cmd/apps/init.go b/cmd/apps/init.go index 1d5c30efb5..08dad714ed 100644 --- a/cmd/apps/init.go +++ b/cmd/apps/init.go @@ -13,6 +13,7 @@ import ( "github.com/charmbracelet/huh" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/experimental/aitools/lib/agents" "github.com/databricks/cli/libs/apps/generator" "github.com/databricks/cli/libs/apps/initializer" "github.com/databricks/cli/libs/apps/manifest" @@ -829,6 +830,11 @@ func runCreate(ctx context.Context, opts createOptions) error { prompt.PrintSuccess(ctx, opts.name, absOutputDir, fileCount, "") } + // Recommend skills installation if coding agents are detected without skills. + if err := agents.RecommendSkillsInstall(ctx); err != nil { + log.Warnf(ctx, "Skills recommendation failed: %v", err) + } + // Execute post-creation actions (deploy and/or run) if shouldDeploy || runMode != prompt.RunModeNone { // Change to project directory for subsequent commands diff --git a/experimental/aitools/lib/agents/recommend.go b/experimental/aitools/lib/agents/recommend.go new file mode 100644 index 0000000000..49448296bf --- /dev/null +++ b/experimental/aitools/lib/agents/recommend.go @@ -0,0 +1,48 @@ +package agents + +import ( + "context" + "fmt" + "os" + "os/exec" + + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/log" +) + +// RecommendSkillsInstall checks if coding agents are detected but have no skills installed. +// In interactive mode, prompts the user to install now. In non-interactive mode, prints a hint. +func RecommendSkillsInstall(ctx context.Context) error { + if HasDatabricksSkillsInstalled() { + return nil + } + + if !cmdio.IsPromptSupported(ctx) { + cmdio.LogString(ctx, "Tip: coding agents detected without Databricks skills. Run 'databricks experimental aitools skills install' to install them.") + return nil + } + + yes, err := cmdio.AskYesOrNo(ctx, "Coding agents detected without Databricks skills. Install skills now?") + if err != nil { + return err + } + if !yes { + return nil + } + + executable, err := os.Executable() + if err != nil { + return fmt.Errorf("failed to get executable path: %w", err) + } + + cmd := exec.CommandContext(ctx, executable, "experimental", "aitools", "skills", "install") + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Stdin = os.Stdin + + if err := cmd.Run(); err != nil { + log.Warnf(ctx, "Skills installation failed: %v", err) + } + + return nil +} diff --git a/experimental/aitools/lib/agents/recommend_test.go b/experimental/aitools/lib/agents/recommend_test.go new file mode 100644 index 0000000000..52502274d6 --- /dev/null +++ b/experimental/aitools/lib/agents/recommend_test.go @@ -0,0 +1,92 @@ +package agents + +import ( + "context" + "io" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/libs/cmdio" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRecommendSkillsInstallSkipsWhenSkillsExist(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "databricks"), 0o755)) + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func() (string, error) { return tmpDir, nil }, + }, + } + defer func() { Registry = origRegistry }() + + ctx := cmdio.MockDiscard(context.Background()) + err := RecommendSkillsInstall(ctx) + assert.NoError(t, err) +} + +func TestRecommendSkillsInstallSkipsWhenNoAgents(t *testing.T) { + origRegistry := Registry + Registry = []Agent{} + defer func() { Registry = origRegistry }() + + ctx := cmdio.MockDiscard(context.Background()) + err := RecommendSkillsInstall(ctx) + assert.NoError(t, err) +} + +func TestRecommendSkillsInstallNonInteractive(t *testing.T) { + tmpDir := t.TempDir() + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func() (string, error) { return tmpDir, nil }, + }, + } + defer func() { Registry = origRegistry }() + + ctx, stderr := cmdio.NewTestContextWithStderr(context.Background()) + err := RecommendSkillsInstall(ctx) + require.NoError(t, err) + assert.Contains(t, stderr.String(), "databricks experimental aitools skills install") +} + +func TestRecommendSkillsInstallInteractiveDecline(t *testing.T) { + tmpDir := t.TempDir() + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func() (string, error) { return tmpDir, nil }, + }, + } + defer func() { Registry = origRegistry }() + + ctx, testIO := cmdio.SetupTest(context.Background(), cmdio.TestOptions{PromptSupported: true}) + defer testIO.Done() + + // Drain stderr so the prompt write doesn't block. + go func() { _, _ = io.Copy(io.Discard, testIO.Stderr) }() + + errc := make(chan error, 1) + go func() { + errc <- RecommendSkillsInstall(ctx) + }() + + _, err := testIO.Stdin.WriteString("n\n") + require.NoError(t, err) + require.NoError(t, testIO.Stdin.Flush()) + + assert.NoError(t, <-errc) +} diff --git a/experimental/aitools/lib/agents/skills.go b/experimental/aitools/lib/agents/skills.go new file mode 100644 index 0000000000..ae609c13e1 --- /dev/null +++ b/experimental/aitools/lib/agents/skills.go @@ -0,0 +1,58 @@ +package agents + +import ( + "os" + "path/filepath" + "strings" +) + +const ( + // databricksSkillPrefix is the prefix used by Databricks skills (e.g., "databricks", "databricks-apps"). + databricksSkillPrefix = "databricks" + + // canonicalSkillsDir is the shared location for skills when multiple agents are detected. + canonicalSkillsDir = ".databricks/agent-skills" +) + +// HasDatabricksSkillsInstalled checks if at least one detected agent has Databricks skills installed. +// Returns true if no agents are detected (nothing to recommend) or if any agent has Databricks skills. +func HasDatabricksSkillsInstalled() bool { + installed := DetectInstalled() + if len(installed) == 0 { + return true + } + + // Check canonical location first (~/.databricks/agent-skills/). + homeDir, err := getHomeDir() + if err == nil { + if hasDatabricksSkillsIn(filepath.Join(homeDir, canonicalSkillsDir)) { + return true + } + } + + // Check each agent's skills directory. + for _, agent := range installed { + dir, err := agent.SkillsDir() + if err != nil { + continue + } + if hasDatabricksSkillsIn(dir) { + return true + } + } + return false +} + +// hasDatabricksSkillsIn checks if dir contains a subdirectory starting with "databricks". +func hasDatabricksSkillsIn(dir string) bool { + entries, err := os.ReadDir(dir) + if err != nil { + return false + } + for _, e := range entries { + if e.IsDir() && strings.HasPrefix(e.Name(), databricksSkillPrefix) { + return true + } + } + return false +} diff --git a/experimental/aitools/lib/agents/skills_test.go b/experimental/aitools/lib/agents/skills_test.go new file mode 100644 index 0000000000..a45a53fbcd --- /dev/null +++ b/experimental/aitools/lib/agents/skills_test.go @@ -0,0 +1,129 @@ +package agents + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHasDatabricksSkillsInstalledNoAgents(t *testing.T) { + origRegistry := Registry + Registry = []Agent{} + defer func() { Registry = origRegistry }() + + assert.True(t, HasDatabricksSkillsInstalled()) +} + +func TestHasDatabricksSkillsInstalledWithDatabricksSkill(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "databricks"), 0o755)) + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func() (string, error) { return tmpDir, nil }, + }, + } + defer func() { Registry = origRegistry }() + + assert.True(t, HasDatabricksSkillsInstalled()) +} + +func TestHasDatabricksSkillsInstalledWithDatabricksAppsSkill(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "databricks-apps"), 0o755)) + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func() (string, error) { return tmpDir, nil }, + }, + } + defer func() { Registry = origRegistry }() + + assert.True(t, HasDatabricksSkillsInstalled()) +} + +func TestHasDatabricksSkillsInstalledWithOnlyNonDatabricksSkills(t *testing.T) { + tmpDir := t.TempDir() + // Non-databricks skills should not count. + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "mcp-builder"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "rust-webapp"), 0o755)) + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func() (string, error) { return tmpDir, nil }, + }, + } + defer func() { Registry = origRegistry }() + + assert.False(t, HasDatabricksSkillsInstalled()) +} + +func TestHasDatabricksSkillsInstalledNoSkillsDir(t *testing.T) { + tmpDir := t.TempDir() + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func() (string, error) { return tmpDir, nil }, + }, + } + defer func() { Registry = origRegistry }() + + assert.False(t, HasDatabricksSkillsInstalled()) +} + +func TestHasDatabricksSkillsInstalledCustomSubdir(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "global_skills", "databricks"), 0o755)) + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func() (string, error) { return tmpDir, nil }, + SkillsSubdir: "global_skills", + }, + } + defer func() { Registry = origRegistry }() + + assert.True(t, HasDatabricksSkillsInstalled()) +} + +func TestHasDatabricksSkillsInstalledCanonicalLocation(t *testing.T) { + tmpHome := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, canonicalSkillsDir, "databricks"), 0o755)) + + // Agent detected but no skills in agent's own dir. + agentDir := filepath.Join(tmpHome, ".claude") + require.NoError(t, os.MkdirAll(agentDir, 0o755)) + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func() (string, error) { return agentDir, nil }, + }, + } + defer func() { Registry = origRegistry }() + + // Override home dir via env for the canonical path check. + t.Setenv("HOME", tmpHome) + + assert.True(t, HasDatabricksSkillsInstalled()) +} From 468619fc1e35eb3fb969215ad6e7c50408cd8cfa Mon Sep 17 00:00:00 2001 From: Arseny Kravchenko Date: Wed, 18 Feb 2026 13:03:16 +0100 Subject: [PATCH 2/3] Extract skills install logic into lib/installer package Co-Authored-By: Claude Opus 4.6 --- cmd/apps/init.go | 8 +- experimental/aitools/cmd/skills.go | 276 +---------------- experimental/aitools/lib/agents/recommend.go | 19 +- .../aitools/lib/agents/recommend_test.go | 15 +- experimental/aitools/lib/agents/skills.go | 6 +- .../aitools/lib/agents/skills_test.go | 4 +- .../aitools/lib/installer/installer.go | 277 ++++++++++++++++++ 7 files changed, 308 insertions(+), 297 deletions(-) create mode 100644 experimental/aitools/lib/installer/installer.go diff --git a/cmd/apps/init.go b/cmd/apps/init.go index 08dad714ed..199cde4e24 100644 --- a/cmd/apps/init.go +++ b/cmd/apps/init.go @@ -14,6 +14,7 @@ import ( "github.com/charmbracelet/huh" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/experimental/aitools/lib/installer" "github.com/databricks/cli/libs/apps/generator" "github.com/databricks/cli/libs/apps/initializer" "github.com/databricks/cli/libs/apps/manifest" @@ -831,7 +832,12 @@ func runCreate(ctx context.Context, opts createOptions) error { } // Recommend skills installation if coding agents are detected without skills. - if err := agents.RecommendSkillsInstall(ctx); err != nil { + // In flags mode, only print a hint — never prompt interactively. + if flagsMode { + if !agents.HasDatabricksSkillsInstalled() { + cmdio.LogString(ctx, "Tip: coding agents detected without Databricks skills. Run 'databricks experimental aitools skills install' to install them.") + } + } else if err := agents.RecommendSkillsInstall(ctx, installer.InstallAllSkills); err != nil { log.Warnf(ctx, "Skills recommendation failed: %v", err) } diff --git a/experimental/aitools/cmd/skills.go b/experimental/aitools/cmd/skills.go index 646fb61367..c08de15ed2 100644 --- a/experimental/aitools/cmd/skills.go +++ b/experimental/aitools/cmd/skills.go @@ -1,48 +1,10 @@ package mcp import ( - "context" - "encoding/json" - "fmt" - "io" - "net/http" - "os" - "path/filepath" - "time" - - "github.com/databricks/cli/experimental/aitools/lib/agents" - "github.com/databricks/cli/libs/cmdio" - "github.com/fatih/color" + "github.com/databricks/cli/experimental/aitools/lib/installer" "github.com/spf13/cobra" ) -const ( - skillsRepoOwner = "databricks" - skillsRepoName = "databricks-agent-skills" - skillsRepoPath = "skills" - defaultSkillsRepoBranch = "main" - canonicalSkillsDir = ".databricks/agent-skills" // canonical location for symlink source -) - -func getSkillsBranch() string { - if branch := os.Getenv("DATABRICKS_SKILLS_BRANCH"); branch != "" { - return branch - } - return defaultSkillsRepoBranch -} - -type Manifest struct { - Version string `json:"version"` - UpdatedAt string `json:"updated_at"` - Skills map[string]SkillMeta `json:"skills"` -} - -type SkillMeta struct { - Version string `json:"version"` - UpdatedAt string `json:"updated_at"` - Files []string `json:"files"` -} - func newSkillsCmd() *cobra.Command { cmd := &cobra.Command{ Use: "skills", @@ -61,7 +23,7 @@ func newSkillsListCmd() *cobra.Command { Use: "list", Short: "List available skills", RunE: func(cmd *cobra.Command, args []string) error { - return listSkills(cmd.Context()) + return installer.ListSkills(cmd.Context()) }, } } @@ -79,239 +41,9 @@ and symlinked to each agent to avoid duplication. Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity`, RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { - return installSkill(cmd.Context(), args[0]) + return installer.InstallSkill(cmd.Context(), args[0]) } - return installAllSkills(cmd.Context()) + return installer.InstallAllSkills(cmd.Context()) }, } } - -func fetchManifest(ctx context.Context) (*Manifest, error) { - url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/manifest.json", - skillsRepoOwner, skillsRepoName, getSkillsBranch()) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) - } - - client := &http.Client{Timeout: 30 * time.Second} - resp, err := client.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to fetch manifest: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("failed to fetch manifest: HTTP %d", resp.StatusCode) - } - - var manifest Manifest - if err := json.NewDecoder(resp.Body).Decode(&manifest); err != nil { - return nil, fmt.Errorf("failed to parse manifest: %w", err) - } - - return &manifest, nil -} - -func fetchSkillFile(ctx context.Context, skillName, filePath string) ([]byte, error) { - url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s/%s/%s", - skillsRepoOwner, skillsRepoName, getSkillsBranch(), skillsRepoPath, skillName, filePath) - - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) - } - - client := &http.Client{Timeout: 30 * time.Second} - resp, err := client.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to fetch %s: %w", filePath, err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("failed to fetch %s: HTTP %d", filePath, resp.StatusCode) - } - - return io.ReadAll(resp.Body) -} - -func listSkills(ctx context.Context) error { - manifest, err := fetchManifest(ctx) - if err != nil { - return err - } - - cmdio.LogString(ctx, "Available skills:") - cmdio.LogString(ctx, "") - - for name, meta := range manifest.Skills { - cmdio.LogString(ctx, fmt.Sprintf(" %s (v%s)", name, meta.Version)) - } - - cmdio.LogString(ctx, "") - cmdio.LogString(ctx, "Install all with: databricks experimental aitools skills install") - cmdio.LogString(ctx, "Install one with: databricks experimental aitools skills install ") - return nil -} - -func installAllSkills(ctx context.Context) error { - manifest, err := fetchManifest(ctx) - if err != nil { - return err - } - - // detect agents once for all skills - detectedAgents := agents.DetectInstalled() - if len(detectedAgents) == 0 { - printNoAgentsDetected(ctx) - return nil - } - - printDetectedAgents(ctx, detectedAgents) - - for name, meta := range manifest.Skills { - if err := installSkillForAgents(ctx, name, meta.Files, detectedAgents); err != nil { - return err - } - } - return nil -} - -func printNoAgentsDetected(ctx context.Context) { - cmdio.LogString(ctx, color.YellowString("No supported coding agents detected.")) - cmdio.LogString(ctx, "") - cmdio.LogString(ctx, "Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity") - cmdio.LogString(ctx, "Please install at least one coding agent first.") -} - -func printDetectedAgents(ctx context.Context, detectedAgents []*agents.Agent) { - cmdio.LogString(ctx, "Detected coding agents:") - for _, agent := range detectedAgents { - cmdio.LogString(ctx, " - "+agent.DisplayName) - } - cmdio.LogString(ctx, "") -} - -func installSkill(ctx context.Context, skillName string) error { - manifest, err := fetchManifest(ctx) - if err != nil { - return err - } - - if _, ok := manifest.Skills[skillName]; !ok { - return fmt.Errorf("skill %q not found", skillName) - } - - detectedAgents := agents.DetectInstalled() - if len(detectedAgents) == 0 { - printNoAgentsDetected(ctx) - return nil - } - - printDetectedAgents(ctx, detectedAgents) - - return installSkillForAgents(ctx, skillName, manifest.Skills[skillName].Files, detectedAgents) -} - -func installSkillForAgents(ctx context.Context, skillName string, files []string, detectedAgents []*agents.Agent) error { - homeDir, err := os.UserHomeDir() - if err != nil { - return fmt.Errorf("failed to get home directory: %w", err) - } - - // determine installation strategy - useSymlinks := len(detectedAgents) > 1 - var canonicalDir string - - if useSymlinks { - // install to canonical location and symlink to each agent - canonicalDir = filepath.Join(homeDir, canonicalSkillsDir, skillName) - if err := installSkillToDir(ctx, skillName, canonicalDir, files); err != nil { - return err - } - } - - // install/symlink to each agent - for _, agent := range detectedAgents { - agentSkillDir, err := agent.SkillsDir() - if err != nil { - cmdio.LogString(ctx, color.YellowString("⊘ Skipped %s: %v", agent.DisplayName, err)) - continue - } - - destDir := filepath.Join(agentSkillDir, skillName) - - if useSymlinks { - if err := createSymlink(canonicalDir, destDir); err != nil { - // fallback to copy on symlink failure (e.g., Windows without admin) - cmdio.LogString(ctx, color.YellowString(" Symlink failed for %s, copying instead...", agent.DisplayName)) - if err := installSkillToDir(ctx, skillName, destDir, files); err != nil { - cmdio.LogString(ctx, color.YellowString("⊘ Failed to install for %s: %v", agent.DisplayName, err)) - continue - } - } - cmdio.LogString(ctx, color.GreenString("✓ Installed %q for %s (symlinked)", skillName, agent.DisplayName)) - } else { - // single agent - install directly - if err := installSkillToDir(ctx, skillName, destDir, files); err != nil { - cmdio.LogString(ctx, color.YellowString("⊘ Failed to install for %s: %v", agent.DisplayName, err)) - continue - } - cmdio.LogString(ctx, color.GreenString("✓ Installed %q for %s", skillName, agent.DisplayName)) - } - } - - return nil -} - -func installSkillToDir(ctx context.Context, skillName, destDir string, files []string) error { - // remove existing skill directory for clean install - if err := os.RemoveAll(destDir); err != nil { - return fmt.Errorf("failed to remove existing skill: %w", err) - } - - if err := os.MkdirAll(destDir, 0o755); err != nil { - return fmt.Errorf("failed to create directory: %w", err) - } - - // download all files - for _, file := range files { - content, err := fetchSkillFile(ctx, skillName, file) - if err != nil { - return err - } - - destPath := filepath.Join(destDir, file) - - // create parent directories if needed - if err := os.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { - return fmt.Errorf("failed to create directory: %w", err) - } - - if err := os.WriteFile(destPath, content, 0o644); err != nil { - return fmt.Errorf("failed to write %s: %w", file, err) - } - } - - return nil -} - -func createSymlink(source, dest string) error { - // ensure parent directory exists - if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { - return fmt.Errorf("failed to create parent directory: %w", err) - } - - // remove existing symlink or directory - if err := os.RemoveAll(dest); err != nil { - return fmt.Errorf("failed to remove existing path: %w", err) - } - - // create symlink - if err := os.Symlink(source, dest); err != nil { - return fmt.Errorf("failed to create symlink: %w", err) - } - - return nil -} diff --git a/experimental/aitools/lib/agents/recommend.go b/experimental/aitools/lib/agents/recommend.go index 49448296bf..1cf60de10d 100644 --- a/experimental/aitools/lib/agents/recommend.go +++ b/experimental/aitools/lib/agents/recommend.go @@ -2,17 +2,14 @@ package agents import ( "context" - "fmt" - "os" - "os/exec" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/log" ) // RecommendSkillsInstall checks if coding agents are detected but have no skills installed. -// In interactive mode, prompts the user to install now. In non-interactive mode, prints a hint. -func RecommendSkillsInstall(ctx context.Context) error { +// In interactive mode, prompts the user to install now using installFn. In non-interactive mode, prints a hint. +func RecommendSkillsInstall(ctx context.Context, installFn func(context.Context) error) error { if HasDatabricksSkillsInstalled() { return nil } @@ -30,17 +27,7 @@ func RecommendSkillsInstall(ctx context.Context) error { return nil } - executable, err := os.Executable() - if err != nil { - return fmt.Errorf("failed to get executable path: %w", err) - } - - cmd := exec.CommandContext(ctx, executable, "experimental", "aitools", "skills", "install") - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - cmd.Stdin = os.Stdin - - if err := cmd.Run(); err != nil { + if err := installFn(ctx); err != nil { log.Warnf(ctx, "Skills installation failed: %v", err) } diff --git a/experimental/aitools/lib/agents/recommend_test.go b/experimental/aitools/lib/agents/recommend_test.go index 52502274d6..10adfc4107 100644 --- a/experimental/aitools/lib/agents/recommend_test.go +++ b/experimental/aitools/lib/agents/recommend_test.go @@ -12,8 +12,11 @@ import ( "github.com/stretchr/testify/require" ) +func noopInstall(context.Context) error { return nil } + func TestRecommendSkillsInstallSkipsWhenSkillsExist(t *testing.T) { tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "databricks"), 0o755)) origRegistry := Registry @@ -27,22 +30,25 @@ func TestRecommendSkillsInstallSkipsWhenSkillsExist(t *testing.T) { defer func() { Registry = origRegistry }() ctx := cmdio.MockDiscard(context.Background()) - err := RecommendSkillsInstall(ctx) + err := RecommendSkillsInstall(ctx, noopInstall) assert.NoError(t, err) } func TestRecommendSkillsInstallSkipsWhenNoAgents(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + origRegistry := Registry Registry = []Agent{} defer func() { Registry = origRegistry }() ctx := cmdio.MockDiscard(context.Background()) - err := RecommendSkillsInstall(ctx) + err := RecommendSkillsInstall(ctx, noopInstall) assert.NoError(t, err) } func TestRecommendSkillsInstallNonInteractive(t *testing.T) { tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) origRegistry := Registry Registry = []Agent{ @@ -55,13 +61,14 @@ func TestRecommendSkillsInstallNonInteractive(t *testing.T) { defer func() { Registry = origRegistry }() ctx, stderr := cmdio.NewTestContextWithStderr(context.Background()) - err := RecommendSkillsInstall(ctx) + err := RecommendSkillsInstall(ctx, noopInstall) require.NoError(t, err) assert.Contains(t, stderr.String(), "databricks experimental aitools skills install") } func TestRecommendSkillsInstallInteractiveDecline(t *testing.T) { tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) origRegistry := Registry Registry = []Agent{ @@ -81,7 +88,7 @@ func TestRecommendSkillsInstallInteractiveDecline(t *testing.T) { errc := make(chan error, 1) go func() { - errc <- RecommendSkillsInstall(ctx) + errc <- RecommendSkillsInstall(ctx, noopInstall) }() _, err := testIO.Stdin.WriteString("n\n") diff --git a/experimental/aitools/lib/agents/skills.go b/experimental/aitools/lib/agents/skills.go index ae609c13e1..203deb7faf 100644 --- a/experimental/aitools/lib/agents/skills.go +++ b/experimental/aitools/lib/agents/skills.go @@ -10,8 +10,8 @@ const ( // databricksSkillPrefix is the prefix used by Databricks skills (e.g., "databricks", "databricks-apps"). databricksSkillPrefix = "databricks" - // canonicalSkillsDir is the shared location for skills when multiple agents are detected. - canonicalSkillsDir = ".databricks/agent-skills" + // CanonicalSkillsDir is the shared location for skills when multiple agents are detected. + CanonicalSkillsDir = ".databricks/agent-skills" ) // HasDatabricksSkillsInstalled checks if at least one detected agent has Databricks skills installed. @@ -25,7 +25,7 @@ func HasDatabricksSkillsInstalled() bool { // Check canonical location first (~/.databricks/agent-skills/). homeDir, err := getHomeDir() if err == nil { - if hasDatabricksSkillsIn(filepath.Join(homeDir, canonicalSkillsDir)) { + if hasDatabricksSkillsIn(filepath.Join(homeDir, CanonicalSkillsDir)) { return true } } diff --git a/experimental/aitools/lib/agents/skills_test.go b/experimental/aitools/lib/agents/skills_test.go index a45a53fbcd..386381cb2d 100644 --- a/experimental/aitools/lib/agents/skills_test.go +++ b/experimental/aitools/lib/agents/skills_test.go @@ -53,6 +53,7 @@ func TestHasDatabricksSkillsInstalledWithDatabricksAppsSkill(t *testing.T) { func TestHasDatabricksSkillsInstalledWithOnlyNonDatabricksSkills(t *testing.T) { tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) // Non-databricks skills should not count. require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "mcp-builder"), 0o755)) require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "rust-webapp"), 0o755)) @@ -72,6 +73,7 @@ func TestHasDatabricksSkillsInstalledWithOnlyNonDatabricksSkills(t *testing.T) { func TestHasDatabricksSkillsInstalledNoSkillsDir(t *testing.T) { tmpDir := t.TempDir() + t.Setenv("HOME", tmpDir) origRegistry := Registry Registry = []Agent{ @@ -106,7 +108,7 @@ func TestHasDatabricksSkillsInstalledCustomSubdir(t *testing.T) { func TestHasDatabricksSkillsInstalledCanonicalLocation(t *testing.T) { tmpHome := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, canonicalSkillsDir, "databricks"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, CanonicalSkillsDir, "databricks"), 0o755)) // Agent detected but no skills in agent's own dir. agentDir := filepath.Join(tmpHome, ".claude") diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go new file mode 100644 index 0000000000..c77899d5fc --- /dev/null +++ b/experimental/aitools/lib/installer/installer.go @@ -0,0 +1,277 @@ +package installer + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "time" + + "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/libs/cmdio" + "github.com/fatih/color" +) + +const ( + skillsRepoOwner = "databricks" + skillsRepoName = "databricks-agent-skills" + skillsRepoPath = "skills" + defaultSkillsRepoBranch = "main" +) + +func getSkillsBranch() string { + if branch := os.Getenv("DATABRICKS_SKILLS_BRANCH"); branch != "" { + return branch + } + return defaultSkillsRepoBranch +} + +// Manifest describes the skills manifest fetched from the skills repo. +type Manifest struct { + Version string `json:"version"` + UpdatedAt string `json:"updated_at"` + Skills map[string]SkillMeta `json:"skills"` +} + +// SkillMeta describes a single skill entry in the manifest. +type SkillMeta struct { + Version string `json:"version"` + UpdatedAt string `json:"updated_at"` + Files []string `json:"files"` +} + +// FetchManifest fetches the skills manifest from the skills repo. +func FetchManifest(ctx context.Context) (*Manifest, error) { + url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/manifest.json", + skillsRepoOwner, skillsRepoName, getSkillsBranch()) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + client := &http.Client{Timeout: 30 * time.Second} + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to fetch manifest: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("failed to fetch manifest: HTTP %d", resp.StatusCode) + } + + var manifest Manifest + if err := json.NewDecoder(resp.Body).Decode(&manifest); err != nil { + return nil, fmt.Errorf("failed to parse manifest: %w", err) + } + + return &manifest, nil +} + +func fetchSkillFile(ctx context.Context, skillName, filePath string) ([]byte, error) { + url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s/%s/%s", + skillsRepoOwner, skillsRepoName, getSkillsBranch(), skillsRepoPath, skillName, filePath) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + client := &http.Client{Timeout: 30 * time.Second} + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to fetch %s: %w", filePath, err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("failed to fetch %s: HTTP %d", filePath, resp.StatusCode) + } + + return io.ReadAll(resp.Body) +} + +// ListSkills fetches and prints available skills. +func ListSkills(ctx context.Context) error { + manifest, err := FetchManifest(ctx) + if err != nil { + return err + } + + cmdio.LogString(ctx, "Available skills:") + cmdio.LogString(ctx, "") + + for name, meta := range manifest.Skills { + cmdio.LogString(ctx, fmt.Sprintf(" %s (v%s)", name, meta.Version)) + } + + cmdio.LogString(ctx, "") + cmdio.LogString(ctx, "Install all with: databricks experimental aitools skills install") + cmdio.LogString(ctx, "Install one with: databricks experimental aitools skills install ") + return nil +} + +// InstallAllSkills fetches the manifest and installs all skills for detected agents. +func InstallAllSkills(ctx context.Context) error { + manifest, err := FetchManifest(ctx) + if err != nil { + return err + } + + detectedAgents := agents.DetectInstalled() + if len(detectedAgents) == 0 { + printNoAgentsDetected(ctx) + return nil + } + + printDetectedAgents(ctx, detectedAgents) + + for name, meta := range manifest.Skills { + if err := installSkillForAgents(ctx, name, meta.Files, detectedAgents); err != nil { + return err + } + } + return nil +} + +// InstallSkill fetches the manifest and installs a single skill by name. +func InstallSkill(ctx context.Context, skillName string) error { + manifest, err := FetchManifest(ctx) + if err != nil { + return err + } + + if _, ok := manifest.Skills[skillName]; !ok { + return fmt.Errorf("skill %q not found", skillName) + } + + detectedAgents := agents.DetectInstalled() + if len(detectedAgents) == 0 { + printNoAgentsDetected(ctx) + return nil + } + + printDetectedAgents(ctx, detectedAgents) + + return installSkillForAgents(ctx, skillName, manifest.Skills[skillName].Files, detectedAgents) +} + +func printNoAgentsDetected(ctx context.Context) { + cmdio.LogString(ctx, color.YellowString("No supported coding agents detected.")) + cmdio.LogString(ctx, "") + cmdio.LogString(ctx, "Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity") + cmdio.LogString(ctx, "Please install at least one coding agent first.") +} + +func printDetectedAgents(ctx context.Context, detectedAgents []*agents.Agent) { + cmdio.LogString(ctx, "Detected coding agents:") + for _, agent := range detectedAgents { + cmdio.LogString(ctx, " - "+agent.DisplayName) + } + cmdio.LogString(ctx, "") +} + +func installSkillForAgents(ctx context.Context, skillName string, files []string, detectedAgents []*agents.Agent) error { + homeDir, err := os.UserHomeDir() + if err != nil { + return fmt.Errorf("failed to get home directory: %w", err) + } + + // determine installation strategy + useSymlinks := len(detectedAgents) > 1 + var canonicalDir string + + if useSymlinks { + // install to canonical location and symlink to each agent + canonicalDir = filepath.Join(homeDir, agents.CanonicalSkillsDir, skillName) + if err := installSkillToDir(ctx, skillName, canonicalDir, files); err != nil { + return err + } + } + + // install/symlink to each agent + for _, agent := range detectedAgents { + agentSkillDir, err := agent.SkillsDir() + if err != nil { + cmdio.LogString(ctx, color.YellowString("⊘ Skipped %s: %v", agent.DisplayName, err)) + continue + } + + destDir := filepath.Join(agentSkillDir, skillName) + + if useSymlinks { + if err := createSymlink(canonicalDir, destDir); err != nil { + // fallback to copy on symlink failure (e.g., Windows without admin) + cmdio.LogString(ctx, color.YellowString(" Symlink failed for %s, copying instead...", agent.DisplayName)) + if err := installSkillToDir(ctx, skillName, destDir, files); err != nil { + cmdio.LogString(ctx, color.YellowString("⊘ Failed to install for %s: %v", agent.DisplayName, err)) + continue + } + } + cmdio.LogString(ctx, color.GreenString("✓ Installed %q for %s (symlinked)", skillName, agent.DisplayName)) + } else { + // single agent - install directly + if err := installSkillToDir(ctx, skillName, destDir, files); err != nil { + cmdio.LogString(ctx, color.YellowString("⊘ Failed to install for %s: %v", agent.DisplayName, err)) + continue + } + cmdio.LogString(ctx, color.GreenString("✓ Installed %q for %s", skillName, agent.DisplayName)) + } + } + + return nil +} + +func installSkillToDir(ctx context.Context, skillName, destDir string, files []string) error { + // remove existing skill directory for clean install + if err := os.RemoveAll(destDir); err != nil { + return fmt.Errorf("failed to remove existing skill: %w", err) + } + + if err := os.MkdirAll(destDir, 0o755); err != nil { + return fmt.Errorf("failed to create directory: %w", err) + } + + // download all files + for _, file := range files { + content, err := fetchSkillFile(ctx, skillName, file) + if err != nil { + return err + } + + destPath := filepath.Join(destDir, file) + + // create parent directories if needed + if err := os.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { + return fmt.Errorf("failed to create directory: %w", err) + } + + if err := os.WriteFile(destPath, content, 0o644); err != nil { + return fmt.Errorf("failed to write %s: %w", file, err) + } + } + + return nil +} + +func createSymlink(source, dest string) error { + // ensure parent directory exists + if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { + return fmt.Errorf("failed to create parent directory: %w", err) + } + + // remove existing symlink or directory + if err := os.RemoveAll(dest); err != nil { + return fmt.Errorf("failed to remove existing path: %w", err) + } + + // create symlink + if err := os.Symlink(source, dest); err != nil { + return fmt.Errorf("failed to create symlink: %w", err) + } + + return nil +} From 7dc5bfb42bbbf4f01339b6361e44d82e5cc351c6 Mon Sep 17 00:00:00 2001 From: Arseny Kravchenko Date: Thu, 19 Feb 2026 13:36:53 +0100 Subject: [PATCH 3/3] Check only canonical location for skills, backup 3rd-party installs Co-Authored-By: Claude Opus 4.6 --- .../aitools/lib/agents/recommend_test.go | 8 +- experimental/aitools/lib/agents/skills.go | 25 +--- .../aitools/lib/agents/skills_test.go | 44 +++---- .../aitools/lib/installer/installer.go | 57 +++++++-- .../aitools/lib/installer/installer_test.go | 112 ++++++++++++++++++ 5 files changed, 194 insertions(+), 52 deletions(-) create mode 100644 experimental/aitools/lib/installer/installer_test.go diff --git a/experimental/aitools/lib/agents/recommend_test.go b/experimental/aitools/lib/agents/recommend_test.go index 10adfc4107..208711c94c 100644 --- a/experimental/aitools/lib/agents/recommend_test.go +++ b/experimental/aitools/lib/agents/recommend_test.go @@ -17,14 +17,18 @@ func noopInstall(context.Context) error { return nil } func TestRecommendSkillsInstallSkipsWhenSkillsExist(t *testing.T) { tmpDir := t.TempDir() t.Setenv("HOME", tmpDir) - require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "databricks"), 0o755)) + // Skills must be in canonical location to be detected. + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, CanonicalSkillsDir, "databricks"), 0o755)) + + agentDir := filepath.Join(tmpDir, ".claude") + require.NoError(t, os.MkdirAll(agentDir, 0o755)) origRegistry := Registry Registry = []Agent{ { Name: "test-agent", DisplayName: "Test Agent", - ConfigDir: func() (string, error) { return tmpDir, nil }, + ConfigDir: func() (string, error) { return agentDir, nil }, }, } defer func() { Registry = origRegistry }() diff --git a/experimental/aitools/lib/agents/skills.go b/experimental/aitools/lib/agents/skills.go index 203deb7faf..a3eceee7bd 100644 --- a/experimental/aitools/lib/agents/skills.go +++ b/experimental/aitools/lib/agents/skills.go @@ -14,33 +14,20 @@ const ( CanonicalSkillsDir = ".databricks/agent-skills" ) -// HasDatabricksSkillsInstalled checks if at least one detected agent has Databricks skills installed. -// Returns true if no agents are detected (nothing to recommend) or if any agent has Databricks skills. +// HasDatabricksSkillsInstalled checks if Databricks skills are installed in the canonical location. +// Returns true if no agents are detected (nothing to recommend) or if skills exist in ~/.databricks/agent-skills/. +// Only the canonical location is checked so that skills installed by other tools are not mistaken for a proper installation. func HasDatabricksSkillsInstalled() bool { installed := DetectInstalled() if len(installed) == 0 { return true } - // Check canonical location first (~/.databricks/agent-skills/). homeDir, err := getHomeDir() - if err == nil { - if hasDatabricksSkillsIn(filepath.Join(homeDir, CanonicalSkillsDir)) { - return true - } - } - - // Check each agent's skills directory. - for _, agent := range installed { - dir, err := agent.SkillsDir() - if err != nil { - continue - } - if hasDatabricksSkillsIn(dir) { - return true - } + if err != nil { + return false } - return false + return hasDatabricksSkillsIn(filepath.Join(homeDir, CanonicalSkillsDir)) } // hasDatabricksSkillsIn checks if dir contains a subdirectory starting with "databricks". diff --git a/experimental/aitools/lib/agents/skills_test.go b/experimental/aitools/lib/agents/skills_test.go index 386381cb2d..fdb6acde6d 100644 --- a/experimental/aitools/lib/agents/skills_test.go +++ b/experimental/aitools/lib/agents/skills_test.go @@ -17,16 +17,17 @@ func TestHasDatabricksSkillsInstalledNoAgents(t *testing.T) { assert.True(t, HasDatabricksSkillsInstalled()) } -func TestHasDatabricksSkillsInstalledWithDatabricksSkill(t *testing.T) { - tmpDir := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "databricks"), 0o755)) +func TestHasDatabricksSkillsInstalledCanonicalOnly(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, CanonicalSkillsDir, "databricks"), 0o755)) origRegistry := Registry Registry = []Agent{ { Name: "test-agent", DisplayName: "Test Agent", - ConfigDir: func() (string, error) { return tmpDir, nil }, + ConfigDir: func() (string, error) { return filepath.Join(tmpHome, ".claude"), nil }, }, } defer func() { Registry = origRegistry }() @@ -34,21 +35,24 @@ func TestHasDatabricksSkillsInstalledWithDatabricksSkill(t *testing.T) { assert.True(t, HasDatabricksSkillsInstalled()) } -func TestHasDatabricksSkillsInstalledWithDatabricksAppsSkill(t *testing.T) { - tmpDir := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "skills", "databricks-apps"), 0o755)) +func TestHasDatabricksSkillsInstalledIgnoresAgentDir(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + // Skills in agent dir only (e.g., installed by another tool) should not count. + agentDir := filepath.Join(tmpHome, ".claude") + require.NoError(t, os.MkdirAll(filepath.Join(agentDir, "skills", "databricks"), 0o755)) origRegistry := Registry Registry = []Agent{ { Name: "test-agent", DisplayName: "Test Agent", - ConfigDir: func() (string, error) { return tmpDir, nil }, + ConfigDir: func() (string, error) { return agentDir, nil }, }, } defer func() { Registry = origRegistry }() - assert.True(t, HasDatabricksSkillsInstalled()) + assert.False(t, HasDatabricksSkillsInstalled()) } func TestHasDatabricksSkillsInstalledWithOnlyNonDatabricksSkills(t *testing.T) { @@ -88,29 +92,32 @@ func TestHasDatabricksSkillsInstalledNoSkillsDir(t *testing.T) { assert.False(t, HasDatabricksSkillsInstalled()) } -func TestHasDatabricksSkillsInstalledCustomSubdir(t *testing.T) { - tmpDir := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "global_skills", "databricks"), 0o755)) +func TestHasDatabricksSkillsInstalledCustomSubdirNotChecked(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + // Skills in agent's custom subdir should not count — only canonical matters. + require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, ".gemini", "antigravity", "global_skills", "databricks"), 0o755)) origRegistry := Registry Registry = []Agent{ { Name: "test-agent", DisplayName: "Test Agent", - ConfigDir: func() (string, error) { return tmpDir, nil }, + ConfigDir: func() (string, error) { return filepath.Join(tmpHome, ".gemini", "antigravity"), nil }, SkillsSubdir: "global_skills", }, } defer func() { Registry = origRegistry }() - assert.True(t, HasDatabricksSkillsInstalled()) + assert.False(t, HasDatabricksSkillsInstalled()) } -func TestHasDatabricksSkillsInstalledCanonicalLocation(t *testing.T) { +func TestHasDatabricksSkillsInstalledDatabricksAppsCanonical(t *testing.T) { tmpHome := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, CanonicalSkillsDir, "databricks"), 0o755)) + t.Setenv("HOME", tmpHome) + // databricks-apps prefix should match in canonical location. + require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, CanonicalSkillsDir, "databricks-apps"), 0o755)) - // Agent detected but no skills in agent's own dir. agentDir := filepath.Join(tmpHome, ".claude") require.NoError(t, os.MkdirAll(agentDir, 0o755)) @@ -124,8 +131,5 @@ func TestHasDatabricksSkillsInstalledCanonicalLocation(t *testing.T) { } defer func() { Registry = origRegistry }() - // Override home dir via env for the canonical path check. - t.Setenv("HOME", tmpHome) - assert.True(t, HasDatabricksSkillsInstalled()) } diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go index c77899d5fc..a9c1a42b88 100644 --- a/experimental/aitools/lib/installer/installer.go +++ b/experimental/aitools/lib/installer/installer.go @@ -180,18 +180,14 @@ func installSkillForAgents(ctx context.Context, skillName string, files []string return fmt.Errorf("failed to get home directory: %w", err) } - // determine installation strategy - useSymlinks := len(detectedAgents) > 1 - var canonicalDir string - - if useSymlinks { - // install to canonical location and symlink to each agent - canonicalDir = filepath.Join(homeDir, agents.CanonicalSkillsDir, skillName) - if err := installSkillToDir(ctx, skillName, canonicalDir, files); err != nil { - return err - } + // Always install to canonical location first. + canonicalDir := filepath.Join(homeDir, agents.CanonicalSkillsDir, skillName) + if err := installSkillToDir(ctx, skillName, canonicalDir, files); err != nil { + return err } + useSymlinks := len(detectedAgents) > 1 + // install/symlink to each agent for _, agent := range detectedAgents { agentSkillDir, err := agent.SkillsDir() @@ -202,6 +198,12 @@ func installSkillForAgents(ctx context.Context, skillName string, files []string destDir := filepath.Join(agentSkillDir, skillName) + // Back up existing non-canonical skills before overwriting. + if err := backupThirdPartySkill(ctx, destDir, canonicalDir, skillName, agent.DisplayName); err != nil { + cmdio.LogString(ctx, color.YellowString("⊘ Failed to back up existing skill for %s: %v", agent.DisplayName, err)) + continue + } + if useSymlinks { if err := createSymlink(canonicalDir, destDir); err != nil { // fallback to copy on symlink failure (e.g., Windows without admin) @@ -213,7 +215,7 @@ func installSkillForAgents(ctx context.Context, skillName string, files []string } cmdio.LogString(ctx, color.GreenString("✓ Installed %q for %s (symlinked)", skillName, agent.DisplayName)) } else { - // single agent - install directly + // single agent - copy from canonical if err := installSkillToDir(ctx, skillName, destDir, files); err != nil { cmdio.LogString(ctx, color.YellowString("⊘ Failed to install for %s: %v", agent.DisplayName, err)) continue @@ -225,6 +227,39 @@ func installSkillForAgents(ctx context.Context, skillName string, files []string return nil } +// backupThirdPartySkill moves destDir to a temp directory if it exists and is not +// a symlink pointing to canonicalDir. This preserves skills installed by other tools. +func backupThirdPartySkill(ctx context.Context, destDir, canonicalDir, skillName, agentName string) error { + fi, err := os.Lstat(destDir) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + + // If it's a symlink to our canonical dir, no backup needed. + if fi.Mode()&os.ModeSymlink != 0 { + target, err := os.Readlink(destDir) + if err == nil && target == canonicalDir { + return nil + } + } + + backupDir, err := os.MkdirTemp("", fmt.Sprintf("databricks-skill-backup-%s-*", skillName)) + if err != nil { + return fmt.Errorf("failed to create backup directory: %w", err) + } + + backupDest := filepath.Join(backupDir, skillName) + if err := os.Rename(destDir, backupDest); err != nil { + return fmt.Errorf("failed to move existing skill: %w", err) + } + + cmdio.LogString(ctx, color.YellowString(" Existing %q for %s moved to %s", skillName, agentName, backupDest)) + return nil +} + func installSkillToDir(ctx context.Context, skillName, destDir string, files []string) error { // remove existing skill directory for clean install if err := os.RemoveAll(destDir); err != nil { diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go new file mode 100644 index 0000000000..d570a9853a --- /dev/null +++ b/experimental/aitools/lib/installer/installer_test.go @@ -0,0 +1,112 @@ +package installer + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/libs/cmdio" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBackupThirdPartySkillDestDoesNotExist(t *testing.T) { + ctx := cmdio.MockDiscard(context.Background()) + destDir := filepath.Join(t.TempDir(), "nonexistent") + + err := backupThirdPartySkill(ctx, destDir, "/canonical", "databricks", "Test Agent") + assert.NoError(t, err) +} + +func TestBackupThirdPartySkillSymlinkToCanonical(t *testing.T) { + ctx := cmdio.MockDiscard(context.Background()) + tmp := t.TempDir() + + canonicalDir := filepath.Join(tmp, "canonical", "databricks") + require.NoError(t, os.MkdirAll(canonicalDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(canonicalDir, "skill.md"), []byte("ok"), 0o644)) + + destDir := filepath.Join(tmp, "agent", "skills", "databricks") + require.NoError(t, os.MkdirAll(filepath.Dir(destDir), 0o755)) + require.NoError(t, os.Symlink(canonicalDir, destDir)) + + err := backupThirdPartySkill(ctx, destDir, canonicalDir, "databricks", "Test Agent") + assert.NoError(t, err) + + // Symlink should still be in place. + target, err := os.Readlink(destDir) + require.NoError(t, err) + assert.Equal(t, canonicalDir, target) +} + +func TestBackupThirdPartySkillRegularDir(t *testing.T) { + ctx := cmdio.MockDiscard(context.Background()) + tmp := t.TempDir() + + destDir := filepath.Join(tmp, "agent", "skills", "databricks") + require.NoError(t, os.MkdirAll(destDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(destDir, "custom.md"), []byte("custom"), 0o644)) + + err := backupThirdPartySkill(ctx, destDir, "/some/canonical", "databricks", "Test Agent") + require.NoError(t, err) + + // destDir should no longer exist. + _, err = os.Stat(destDir) + assert.True(t, os.IsNotExist(err)) + + // Backup should contain the original file. + matches, err := filepath.Glob(filepath.Join(os.TempDir(), "databricks-skill-backup-databricks-*", "databricks", "custom.md")) + require.NoError(t, err) + require.NotEmpty(t, matches) + + content, err := os.ReadFile(matches[0]) + require.NoError(t, err) + assert.Equal(t, "custom", string(content)) + + // Clean up backup. + require.NoError(t, os.RemoveAll(filepath.Dir(filepath.Dir(matches[0])))) +} + +func TestBackupThirdPartySkillSymlinkToOtherTarget(t *testing.T) { + ctx := cmdio.MockDiscard(context.Background()) + tmp := t.TempDir() + + otherDir := filepath.Join(tmp, "other", "databricks") + require.NoError(t, os.MkdirAll(otherDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(otherDir, "other.md"), []byte("other"), 0o644)) + + destDir := filepath.Join(tmp, "agent", "skills", "databricks") + require.NoError(t, os.MkdirAll(filepath.Dir(destDir), 0o755)) + require.NoError(t, os.Symlink(otherDir, destDir)) + + canonicalDir := filepath.Join(tmp, "canonical", "databricks") + + err := backupThirdPartySkill(ctx, destDir, canonicalDir, "databricks", "Test Agent") + require.NoError(t, err) + + // destDir (the symlink) should no longer exist. + _, err = os.Lstat(destDir) + assert.True(t, os.IsNotExist(err)) + + // Original target should be untouched. + content, err := os.ReadFile(filepath.Join(otherDir, "other.md")) + require.NoError(t, err) + assert.Equal(t, "other", string(content)) +} + +func TestBackupThirdPartySkillRegularFile(t *testing.T) { + ctx := cmdio.MockDiscard(context.Background()) + tmp := t.TempDir() + + // Edge case: destDir is a file, not a directory. + destDir := filepath.Join(tmp, "agent", "skills", "databricks") + require.NoError(t, os.MkdirAll(filepath.Dir(destDir), 0o755)) + require.NoError(t, os.WriteFile(destDir, []byte("file"), 0o644)) + + err := backupThirdPartySkill(ctx, destDir, "/some/canonical", "databricks", "Test Agent") + require.NoError(t, err) + + _, err = os.Stat(destDir) + assert.True(t, os.IsNotExist(err)) +}