diff --git a/experimental/aitools/cmd/aitools.go b/experimental/aitools/cmd/aitools.go index 3058013dbe..f037ac1a22 100644 --- a/experimental/aitools/cmd/aitools.go +++ b/experimental/aitools/cmd/aitools.go @@ -18,6 +18,10 @@ Provides commands to: } cmd.AddCommand(newInstallCmd()) + cmd.AddCommand(newUpdateCmd()) + cmd.AddCommand(newUninstallCmd()) + cmd.AddCommand(newListCmd()) + cmd.AddCommand(newVersionCmd()) cmd.AddCommand(newSkillsCmd()) cmd.AddCommand(newToolsCmd()) diff --git a/experimental/aitools/cmd/install.go b/experimental/aitools/cmd/install.go index 7bdc86449c..4ab020fd88 100644 --- a/experimental/aitools/cmd/install.go +++ b/experimental/aitools/cmd/install.go @@ -1,18 +1,119 @@ package aitools import ( + "context" + "errors" + "fmt" + "strings" + + "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/databricks/cli/libs/cmdio" + "github.com/fatih/color" "github.com/spf13/cobra" ) func newInstallCmd() *cobra.Command { - return &cobra.Command{ - Use: "install [skill-name]", - Short: "Alias for skills install", - Long: `Alias for "databricks experimental aitools skills install". + var skillsFlag, agentsFlag string + var includeExperimental bool + + cmd := &cobra.Command{ + Use: "install", + Short: "Install AI skills for coding agents", + Long: `Install Databricks AI skills for detected coding agents. + +Skills are installed globally to each agent's skills directory. +When multiple agents are detected, skills are stored in a canonical location +and symlinked to each agent to avoid duplication. -Installs Databricks skills for detected coding agents.`, +Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity`, RunE: func(cmd *cobra.Command, args []string) error { - return runSkillsInstall(cmd.Context(), args) + ctx := cmd.Context() + + // Resolve target agents. + var targetAgents []*agents.Agent + if agentsFlag != "" { + var err error + targetAgents, err = resolveAgentNames(ctx, agentsFlag) + if err != nil { + return err + } + } else { + detected := agents.DetectInstalled(ctx) + if len(detected) == 0 { + printNoAgentsMessage(ctx) + return nil + } + + switch { + case len(detected) == 1: + targetAgents = detected + case cmdio.IsPromptSupported(ctx): + var err error + targetAgents, err = promptAgentSelection(ctx, detected) + if err != nil { + return err + } + default: + targetAgents = detected + } + } + + // Build install options. + opts := installer.InstallOptions{ + IncludeExperimental: includeExperimental, + } + if skillsFlag != "" { + opts.SpecificSkills = strings.Split(skillsFlag, ",") + } + + installer.PrintInstallingFor(ctx, targetAgents) + + src := &installer.GitHubManifestSource{} + return installSkillsForAgentsFn(ctx, src, targetAgents, opts) }, } + + cmd.Flags().StringVar(&skillsFlag, "skills", "", "Specific skills to install (comma-separated)") + cmd.Flags().StringVar(&agentsFlag, "agents", "", "Agents to install for (comma-separated, e.g. claude-code,cursor)") + cmd.Flags().BoolVar(&includeExperimental, "include-experimental", false, "Include experimental skills") + return cmd +} + +// resolveAgentNames parses a comma-separated list of agent names and validates +// them against the registry. Returns an error for unrecognized names. +func resolveAgentNames(ctx context.Context, names string) ([]*agents.Agent, error) { + available := make(map[string]*agents.Agent, len(agents.Registry)) + var availableNames []string + for i := range agents.Registry { + a := &agents.Registry[i] + available[a.Name] = a + availableNames = append(availableNames, a.Name) + } + + var result []*agents.Agent + for _, name := range strings.Split(names, ",") { + name = strings.TrimSpace(name) + if name == "" { + continue + } + agent, ok := available[name] + if !ok { + return nil, fmt.Errorf("unknown agent %q. Available agents: %s", name, strings.Join(availableNames, ", ")) + } + result = append(result, agent) + } + + if len(result) == 0 { + return nil, errors.New("no agents specified") + } + return result, nil +} + +// printNoAgentsMessage prints the "no agents detected" message. +func printNoAgentsMessage(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.") } diff --git a/experimental/aitools/cmd/install_test.go b/experimental/aitools/cmd/install_test.go index 64f97ddf9c..d83fb414db 100644 --- a/experimental/aitools/cmd/install_test.go +++ b/experimental/aitools/cmd/install_test.go @@ -1,75 +1,349 @@ package aitools import ( + "bufio" "context" + "os" + "path/filepath" "testing" - "github.com/spf13/cobra" + "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/databricks/cli/libs/cmdio" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestInstallCommandsDelegateToSkillsInstall(t *testing.T) { - originalInstallAllSkills := installAllSkills - originalInstallSkill := installSkill - t.Cleanup(func() { - installAllSkills = originalInstallAllSkills - installSkill = originalInstallSkill - }) - - tests := []struct { - name string - newCmd func() *cobra.Command - args []string - wantAllCalls int - wantSkillCalls []string - }{ - { - name: "skills install installs all skills", - newCmd: newSkillsInstallCmd, - wantAllCalls: 1, - }, - { - name: "skills install forwards skill name", - newCmd: newSkillsInstallCmd, - args: []string{"bundle/review"}, - wantSkillCalls: []string{"bundle/review"}, - }, - { - name: "top level install installs all skills", - newCmd: newInstallCmd, - wantAllCalls: 1, - }, - { - name: "top level install forwards skill name", - newCmd: newInstallCmd, - args: []string{"bundle/review"}, - wantSkillCalls: []string{"bundle/review"}, - }, +func setupInstallMock(t *testing.T) *[]installCall { + t.Helper() + orig := installSkillsForAgentsFn + t.Cleanup(func() { installSkillsForAgentsFn = orig }) + + var calls []installCall + installSkillsForAgentsFn = func(_ context.Context, _ installer.ManifestSource, targetAgents []*agents.Agent, opts installer.InstallOptions) error { + names := make([]string, len(targetAgents)) + for i, a := range targetAgents { + names[i] = a.Name + } + calls = append(calls, installCall{agents: names, opts: opts}) + return nil } + return &calls +} + +type installCall struct { + agents []string + opts installer.InstallOptions +} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - allCalls := 0 - var skillCalls []string +func setupTestAgents(t *testing.T) string { + t.Helper() + tmp := t.TempDir() + t.Setenv("HOME", tmp) + // Create config dirs for two agents. + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".claude"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".cursor"), 0o755)) + return tmp +} - installAllSkills = func(context.Context) error { - allCalls++ - return nil - } - installSkill = func(_ context.Context, skillName string) error { - skillCalls = append(skillCalls, skillName) - return nil +func TestInstallAllSkillsForAllAgents(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newInstallCmd() + cmd.SetContext(ctx) + + err := cmd.RunE(cmd, nil) + require.NoError(t, err) + + require.Len(t, *calls, 1) + assert.Len(t, (*calls)[0].agents, 2) + assert.Nil(t, (*calls)[0].opts.SpecificSkills) +} + +func TestInstallSpecificSkills(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newInstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--skills", "databricks,databricks-apps"}) + + err := cmd.Execute() + require.NoError(t, err) + + require.Len(t, *calls, 1) + assert.Equal(t, []string{"databricks", "databricks-apps"}, (*calls)[0].opts.SpecificSkills) +} + +func TestInstallSingleSkill(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newInstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--skills", "databricks"}) + + err := cmd.Execute() + require.NoError(t, err) + + require.Len(t, *calls, 1) + assert.Equal(t, []string{"databricks"}, (*calls)[0].opts.SpecificSkills) +} + +func TestInstallSpecificAgents(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newInstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--agents", "claude-code"}) + + err := cmd.Execute() + require.NoError(t, err) + + require.Len(t, *calls, 1) + assert.Equal(t, []string{"claude-code"}, (*calls)[0].agents) +} + +func TestInstallUnknownAgentErrors(t *testing.T) { + setupTestAgents(t) + setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newInstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--agents", "invalid-agent"}) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown agent") + assert.Contains(t, err.Error(), "Available agents:") +} + +func TestInstallIncludeExperimental(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newInstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--include-experimental"}) + + err := cmd.Execute() + require.NoError(t, err) + + require.Len(t, *calls, 1) + assert.True(t, (*calls)[0].opts.IncludeExperimental) +} + +func TestInstallInteractivePrompt(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + origPrompt := promptAgentSelection + t.Cleanup(func() { promptAgentSelection = origPrompt }) + + promptCalled := false + promptAgentSelection = func(_ context.Context, detected []*agents.Agent) ([]*agents.Agent, error) { + promptCalled = true + return detected[:1], nil + } + + ctx, test := cmdio.SetupTest(t.Context(), cmdio.TestOptions{PromptSupported: true}) + defer test.Done() + + drain := func(r *bufio.Reader) { + buf := make([]byte, 4096) + for { + _, err := r.Read(buf) + if err != nil { + return } + } + } + go drain(test.Stdout) + go drain(test.Stderr) + + cmd := newInstallCmd() + cmd.SetContext(ctx) + + err := cmd.RunE(cmd, nil) + require.NoError(t, err) + + assert.True(t, promptCalled, "prompt should be called when 2+ agents detected and interactive") + require.Len(t, *calls, 1) + assert.Len(t, (*calls)[0].agents, 1, "only the selected agent should be passed") +} - cmd := tt.newCmd() - cmd.SetContext(t.Context()) +func TestInstallNonInteractiveUsesAllAgents(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) - err := cmd.RunE(cmd, tt.args) - require.NoError(t, err) + origPrompt := promptAgentSelection + t.Cleanup(func() { promptAgentSelection = origPrompt }) - assert.Equal(t, tt.wantAllCalls, allCalls) - assert.Equal(t, tt.wantSkillCalls, skillCalls) - }) + promptCalled := false + promptAgentSelection = func(_ context.Context, detected []*agents.Agent) ([]*agents.Agent, error) { + promptCalled = true + return detected, nil } + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newInstallCmd() + cmd.SetContext(ctx) + + err := cmd.RunE(cmd, nil) + require.NoError(t, err) + + assert.False(t, promptCalled, "prompt should not be called in non-interactive mode") + require.Len(t, *calls, 1) + assert.Len(t, (*calls)[0].agents, 2, "all detected agents should be used") +} + +func TestInstallNoAgentsDetected(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + + calls := setupInstallMock(t) + ctx := cmdio.MockDiscard(t.Context()) + + cmd := newInstallCmd() + cmd.SetContext(ctx) + + err := cmd.RunE(cmd, nil) + require.NoError(t, err) + assert.Empty(t, *calls, "install should not be called when no agents detected") +} + +func TestInstallAgentsFlagSkipsPrompt(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + origPrompt := promptAgentSelection + t.Cleanup(func() { promptAgentSelection = origPrompt }) + + promptCalled := false + promptAgentSelection = func(_ context.Context, detected []*agents.Agent) ([]*agents.Agent, error) { + promptCalled = true + return detected, nil + } + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newInstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--agents", "claude-code,cursor"}) + + err := cmd.Execute() + require.NoError(t, err) + + assert.False(t, promptCalled, "prompt should not be called when --agents is specified") + require.Len(t, *calls, 1) + assert.Equal(t, []string{"claude-code", "cursor"}, (*calls)[0].agents) +} + +func TestSkillsInstallDelegatesToInstall(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newSkillsInstallCmd() + cmd.SetContext(ctx) + + err := cmd.RunE(cmd, nil) + require.NoError(t, err) + + require.Len(t, *calls, 1) + assert.Len(t, (*calls)[0].agents, 2) +} + +func TestSkillsInstallForwardsSkillName(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newSkillsInstallCmd() + cmd.SetContext(ctx) + + err := cmd.RunE(cmd, []string{"databricks"}) + require.NoError(t, err) + + require.Len(t, *calls, 1) + assert.Equal(t, []string{"databricks"}, (*calls)[0].opts.SpecificSkills) +} + +func TestSkillsInstallExecuteNoArgs(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newSkillsInstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{}) + + err := cmd.Execute() + require.NoError(t, err) + + require.Len(t, *calls, 1) + assert.Len(t, (*calls)[0].agents, 2) + assert.Nil(t, (*calls)[0].opts.SpecificSkills) +} + +func TestSkillsInstallExecuteWithSkillName(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newSkillsInstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"databricks"}) + + err := cmd.Execute() + require.NoError(t, err) + + require.Len(t, *calls, 1) + assert.Equal(t, []string{"databricks"}, (*calls)[0].opts.SpecificSkills) +} + +func TestSkillsInstallExecuteRejectsTwoArgs(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + cmd := newSkillsInstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"a", "b"}) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "accepts at most 1 arg") +} + +func TestResolveAgentNamesValid(t *testing.T) { + ctx := t.Context() + result, err := resolveAgentNames(ctx, "claude-code,cursor") + require.NoError(t, err) + assert.Len(t, result, 2) + assert.Equal(t, "claude-code", result[0].Name) + assert.Equal(t, "cursor", result[1].Name) +} + +func TestResolveAgentNamesUnknown(t *testing.T) { + ctx := t.Context() + _, err := resolveAgentNames(ctx, "claude-code,invalid-agent") + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown agent") + assert.Contains(t, err.Error(), "invalid-agent") +} + +func TestResolveAgentNamesEmpty(t *testing.T) { + ctx := t.Context() + _, err := resolveAgentNames(ctx, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "no agents specified") } diff --git a/experimental/aitools/cmd/list.go b/experimental/aitools/cmd/list.go new file mode 100644 index 0000000000..24ac1c0828 --- /dev/null +++ b/experimental/aitools/cmd/list.go @@ -0,0 +1,102 @@ +package aitools + +import ( + "fmt" + "sort" + "strings" + "text/tabwriter" + + "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/log" + "github.com/spf13/cobra" +) + +// listSkillsFn is the function used to render the skills list. +// It is a package-level var so tests can replace the data-fetching layer. +var listSkillsFn = defaultListSkills + +func newListCmd() *cobra.Command { + var showSkills bool + + cmd := &cobra.Command{ + Use: "list", + Short: "List installed AI tools components", + RunE: func(cmd *cobra.Command, args []string) error { + // Currently skills is the only component, so always show detailed view. + return listSkillsFn(cmd) + }, + } + + cmd.Flags().BoolVar(&showSkills, "skills", false, "Show detailed skills information") + return cmd +} + +func defaultListSkills(cmd *cobra.Command) error { + ctx := cmd.Context() + + src := &installer.GitHubManifestSource{} + latestTag, _, err := src.FetchLatestRelease(ctx) + if err != nil { + return fmt.Errorf("failed to fetch latest release: %w", err) + } + + manifest, err := src.FetchManifest(ctx, latestTag) + if err != nil { + return fmt.Errorf("failed to fetch manifest: %w", err) + } + + globalDir, err := installer.GlobalSkillsDir(ctx) + if err != nil { + return err + } + + state, err := installer.LoadState(globalDir) + if err != nil { + log.Debugf(ctx, "Could not load install state: %v", err) + } + + // Build sorted list of skill names. + names := make([]string, 0, len(manifest.Skills)) + for name := range manifest.Skills { + names = append(names, name) + } + sort.Strings(names) + + version := strings.TrimPrefix(latestTag, "v") + cmdio.LogString(ctx, "Available skills (v"+version+"):") + cmdio.LogString(ctx, "") + + var buf strings.Builder + tw := tabwriter.NewWriter(&buf, 0, 4, 2, ' ', 0) + fmt.Fprintln(tw, " NAME\tVERSION\tINSTALLED") + + installedCount := 0 + for _, name := range names { + meta := manifest.Skills[name] + + tag := "" + if meta.Experimental { + tag = " [experimental]" + } + + installedStr := "not installed" + if state != nil { + if v, ok := state.Skills[name]; ok { + installedCount++ + if v == meta.Version { + installedStr = "v" + v + " (up to date)" + } else { + installedStr = "v" + v + " (update available)" + } + } + } + + fmt.Fprintf(tw, " %s%s\tv%s\t%s\n", name, tag, meta.Version, installedStr) + } + tw.Flush() + cmdio.LogString(ctx, buf.String()) + + cmdio.LogString(ctx, fmt.Sprintf("%d/%d skills installed (global)", installedCount, len(names))) + return nil +} diff --git a/experimental/aitools/cmd/list_test.go b/experimental/aitools/cmd/list_test.go new file mode 100644 index 0000000000..81a304740d --- /dev/null +++ b/experimental/aitools/cmd/list_test.go @@ -0,0 +1,60 @@ +package aitools + +import ( + "testing" + + "github.com/databricks/cli/libs/cmdio" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestListCommandExists(t *testing.T) { + cmd := newListCmd() + assert.Equal(t, "list", cmd.Use) +} + +func TestListCommandCallsListFn(t *testing.T) { + orig := listSkillsFn + t.Cleanup(func() { listSkillsFn = orig }) + + called := false + listSkillsFn = func(cmd *cobra.Command) error { + called = true + return nil + } + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newListCmd() + cmd.SetContext(ctx) + + err := cmd.RunE(cmd, nil) + require.NoError(t, err) + assert.True(t, called) +} + +func TestListCommandHasSkillsFlag(t *testing.T) { + cmd := newListCmd() + f := cmd.Flags().Lookup("skills") + require.NotNil(t, f, "--skills flag should exist") + assert.Equal(t, "false", f.DefValue) +} + +func TestSkillsListDelegatesToListFn(t *testing.T) { + orig := listSkillsFn + t.Cleanup(func() { listSkillsFn = orig }) + + called := false + listSkillsFn = func(cmd *cobra.Command) error { + called = true + return nil + } + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newSkillsListCmd() + cmd.SetContext(ctx) + + err := cmd.RunE(cmd, nil) + require.NoError(t, err) + assert.True(t, called) +} diff --git a/experimental/aitools/cmd/skills.go b/experimental/aitools/cmd/skills.go index 325ee3c1c0..2b21e0e703 100644 --- a/experimental/aitools/cmd/skills.go +++ b/experimental/aitools/cmd/skills.go @@ -2,23 +2,59 @@ package aitools import ( "context" + "errors" + "github.com/charmbracelet/huh" + "github.com/databricks/cli/experimental/aitools/lib/agents" "github.com/databricks/cli/experimental/aitools/lib/installer" "github.com/spf13/cobra" ) +// Package-level vars for testability. var ( - installAllSkills = installer.InstallAllSkills - installSkill = installer.InstallSkill + promptAgentSelection = defaultPromptAgentSelection + installSkillsForAgentsFn = installer.InstallSkillsForAgents ) +func defaultPromptAgentSelection(ctx context.Context, detected []*agents.Agent) ([]*agents.Agent, error) { + options := make([]huh.Option[string], 0, len(detected)) + agentsByName := make(map[string]*agents.Agent, len(detected)) + for _, a := range detected { + options = append(options, huh.NewOption(a.DisplayName, a.Name).Selected(true)) + agentsByName[a.Name] = a + } + + var selected []string + err := huh.NewMultiSelect[string](). + Title("Select coding agents to install skills for"). + Description("space to toggle, enter to confirm"). + Options(options...). + Value(&selected). + Run() + if err != nil { + return nil, err + } + + if len(selected) == 0 { + return nil, errors.New("at least one agent must be selected") + } + + result := make([]*agents.Agent, 0, len(selected)) + for _, name := range selected { + result = append(result, agentsByName[name]) + } + return result, nil +} + func newSkillsCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "skills", - Short: "Manage Databricks skills for coding agents", - Long: `Manage Databricks skills that extend coding agents with Databricks-specific capabilities.`, + Use: "skills", + Hidden: true, + Short: "Manage Databricks skills for coding agents", + Long: `Manage Databricks skills that extend coding agents with Databricks-specific capabilities.`, } + // Subcommands delegate to the flat top-level commands. cmd.AddCommand(newSkillsListCmd()) cmd.AddCommand(newSkillsInstallCmd()) @@ -30,7 +66,7 @@ func newSkillsListCmd() *cobra.Command { Use: "list", Short: "List available skills", RunE: func(cmd *cobra.Command, args []string) error { - return installer.ListSkills(cmd.Context()) + return listSkillsFn(cmd) }, } } @@ -39,23 +75,18 @@ func newSkillsInstallCmd() *cobra.Command { return &cobra.Command{ Use: "install [skill-name]", Short: "Install Databricks skills for detected coding agents", - Long: `Install Databricks skills to all detected coding agents. - -Skills are installed globally to each agent's skills directory. -When multiple agents are detected, skills are stored in a canonical location -and symlinked to each agent to avoid duplication. - -Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity`, + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runSkillsInstall(cmd.Context(), args) + // Delegate to the flat install command's logic. + installCmd := newInstallCmd() + installCmd.SetContext(cmd.Context()) + if len(args) > 0 { + // Pass the skill name as a --skills flag. + installCmd.SetArgs([]string{"--skills", args[0]}) + } else { + installCmd.SetArgs([]string{}) + } + return installCmd.Execute() }, } } - -func runSkillsInstall(ctx context.Context, args []string) error { - if len(args) > 0 { - return installSkill(ctx, args[0]) - } - - return installAllSkills(ctx) -} diff --git a/experimental/aitools/cmd/uninstall.go b/experimental/aitools/cmd/uninstall.go new file mode 100644 index 0000000000..663d305506 --- /dev/null +++ b/experimental/aitools/cmd/uninstall.go @@ -0,0 +1,30 @@ +package aitools + +import ( + "strings" + + "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/spf13/cobra" +) + +func newUninstallCmd() *cobra.Command { + var skillsFlag string + + cmd := &cobra.Command{ + Use: "uninstall", + Short: "Uninstall AI skills", + Long: `Remove installed Databricks AI skills from all coding agents. + +By default, removes all skills. Use --skills to remove specific skills only.`, + RunE: func(cmd *cobra.Command, args []string) error { + opts := installer.UninstallOptions{} + if skillsFlag != "" { + opts.Skills = strings.Split(skillsFlag, ",") + } + return installer.UninstallSkillsOpts(cmd.Context(), opts) + }, + } + + cmd.Flags().StringVar(&skillsFlag, "skills", "", "Specific skills to uninstall (comma-separated)") + return cmd +} diff --git a/experimental/aitools/cmd/update.go b/experimental/aitools/cmd/update.go new file mode 100644 index 0000000000..46d2d92b4c --- /dev/null +++ b/experimental/aitools/cmd/update.go @@ -0,0 +1,54 @@ +package aitools + +import ( + "strings" + + "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/databricks/cli/libs/cmdio" + "github.com/spf13/cobra" +) + +func newUpdateCmd() *cobra.Command { + var check, force, noNew bool + var skillsFlag string + + cmd := &cobra.Command{ + Use: "update", + Short: "Update installed AI skills", + Long: `Update installed Databricks AI skills to the latest release. + +By default, updates all installed skills and auto-installs new skills +from the manifest. Use --no-new to skip new skills, or --check to +preview what would change without downloading.`, + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + installed := agents.DetectInstalled(ctx) + src := &installer.GitHubManifestSource{} + + opts := installer.UpdateOptions{ + Check: check, + Force: force, + NoNew: noNew, + } + if skillsFlag != "" { + opts.Skills = strings.Split(skillsFlag, ",") + } + + result, err := installer.UpdateSkills(ctx, src, installed, opts) + if err != nil { + return err + } + if result != nil && (len(result.Updated) > 0 || len(result.Added) > 0) { + cmdio.LogString(ctx, installer.FormatUpdateResult(result, check)) + } + return nil + }, + } + + cmd.Flags().BoolVar(&check, "check", false, "Show what would be updated without downloading") + cmd.Flags().BoolVar(&force, "force", false, "Re-download even if versions match") + cmd.Flags().BoolVar(&noNew, "no-new", false, "Don't auto-install new skills from manifest") + cmd.Flags().StringVar(&skillsFlag, "skills", "", "Specific skills to update (comma-separated)") + return cmd +} diff --git a/experimental/aitools/cmd/version.go b/experimental/aitools/cmd/version.go new file mode 100644 index 0000000000..f57f62f8a8 --- /dev/null +++ b/experimental/aitools/cmd/version.go @@ -0,0 +1,92 @@ +package aitools + +import ( + "fmt" + "strings" + + "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" + "github.com/spf13/cobra" +) + +func newVersionCmd() *cobra.Command { + var showSkills bool + + cmd := &cobra.Command{ + Use: "version", + Short: "Show installed AI skills version", + RunE: func(cmd *cobra.Command, args []string) error { + // showSkills is accepted for forward-compat but currently + // skills is the only component, so the output is the same. + _ = showSkills + ctx := cmd.Context() + + globalDir, err := installer.GlobalSkillsDir(ctx) + if err != nil { + return err + } + + state, err := installer.LoadState(globalDir) + if err != nil { + return fmt.Errorf("failed to load install state: %w", err) + } + + if state == nil { + cmdio.LogString(ctx, "No Databricks AI Tools components installed.") + cmdio.LogString(ctx, "") + cmdio.LogString(ctx, "Run 'databricks experimental aitools install' to get started.") + return nil + } + + version := strings.TrimPrefix(state.Release, "v") + skillNoun := "skills" + if len(state.Skills) == 1 { + skillNoun = "skill" + } + + // Best-effort staleness check. + if env.Get(ctx, "DATABRICKS_SKILLS_REF") != "" { + cmdio.LogString(ctx, "Databricks AI Tools:") + cmdio.LogString(ctx, fmt.Sprintf(" Skills: v%s (%d %s)", version, len(state.Skills), skillNoun)) + cmdio.LogString(ctx, " Last updated: "+state.LastUpdated.Format("2006-01-02")) + cmdio.LogString(ctx, " Using custom ref: $DATABRICKS_SKILLS_REF") + return nil + } + + src := &installer.GitHubManifestSource{} + latest, authoritative, err := src.FetchLatestRelease(ctx) + if err != nil { + log.Debugf(ctx, "Could not check for updates: %v", err) + authoritative = false + } + + cmdio.LogString(ctx, "Databricks AI Tools:") + + if !authoritative { + cmdio.LogString(ctx, fmt.Sprintf(" Skills: v%s (%d %s)", version, len(state.Skills), skillNoun)) + cmdio.LogString(ctx, " Last updated: "+state.LastUpdated.Format("2006-01-02")) + cmdio.LogString(ctx, " Could not check for latest version.") + return nil + } + + if latest == state.Release { + cmdio.LogString(ctx, fmt.Sprintf(" Skills: v%s (%d %s, up to date)", version, len(state.Skills), skillNoun)) + cmdio.LogString(ctx, " Last updated: "+state.LastUpdated.Format("2006-01-02")) + } else { + latestVersion := strings.TrimPrefix(latest, "v") + cmdio.LogString(ctx, fmt.Sprintf(" Skills: v%s (%d %s)", version, len(state.Skills), skillNoun)) + cmdio.LogString(ctx, " Update available: v"+latestVersion) + cmdio.LogString(ctx, " Last updated: "+state.LastUpdated.Format("2006-01-02")) + cmdio.LogString(ctx, "") + cmdio.LogString(ctx, "Run 'databricks experimental aitools update' to update.") + } + + return nil + }, + } + + cmd.Flags().BoolVar(&showSkills, "skills", false, "Show detailed skills version information") + return cmd +} diff --git a/experimental/aitools/lib/agents/skills.go b/experimental/aitools/lib/agents/skills.go index 3c0d67dfbb..0ba522e356 100644 --- a/experimental/aitools/lib/agents/skills.go +++ b/experimental/aitools/lib/agents/skills.go @@ -14,12 +14,15 @@ const ( databricksSkillPrefix = "databricks" // CanonicalSkillsDir is the shared location for skills when multiple agents are detected. - CanonicalSkillsDir = ".databricks/agent-skills" + CanonicalSkillsDir = ".databricks/aitools/skills" + + // legacySkillsDir is the old canonical location, checked for backward compatibility. + legacySkillsDir = ".databricks/agent-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. +// Returns true if no agents are detected (nothing to recommend) or if skills exist in +// ~/.databricks/aitools/skills/ or the legacy ~/.databricks/agent-skills/. func HasDatabricksSkillsInstalled(ctx context.Context) bool { installed := DetectInstalled(ctx) if len(installed) == 0 { @@ -30,7 +33,8 @@ func HasDatabricksSkillsInstalled(ctx context.Context) bool { if err != nil { return false } - return hasDatabricksSkillsIn(filepath.Join(homeDir, CanonicalSkillsDir)) + return hasDatabricksSkillsIn(filepath.Join(homeDir, CanonicalSkillsDir)) || + hasDatabricksSkillsIn(filepath.Join(homeDir, legacySkillsDir)) } // 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 15baba9c8f..09192d721c 100644 --- a/experimental/aitools/lib/agents/skills_test.go +++ b/experimental/aitools/lib/agents/skills_test.go @@ -134,3 +134,25 @@ func TestHasDatabricksSkillsInstalledDatabricksAppsCanonical(t *testing.T) { assert.True(t, HasDatabricksSkillsInstalled(t.Context())) } + +func TestHasDatabricksSkillsInstalledLegacyPath(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + // Skills only in the legacy location should still be detected. + require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, legacySkillsDir, "databricks"), 0o755)) + + agentDir := filepath.Join(tmpHome, ".claude") + require.NoError(t, os.MkdirAll(agentDir, 0o755)) + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func(_ context.Context) (string, error) { return agentDir, nil }, + }, + } + defer func() { Registry = origRegistry }() + + assert.True(t, HasDatabricksSkillsInstalled(t.Context())) +} diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go index 084e2dc973..146a68ab5d 100644 --- a/experimental/aitools/lib/installer/installer.go +++ b/experimental/aitools/lib/installer/installer.go @@ -2,19 +2,21 @@ package installer import ( "context" - "encoding/json" "fmt" "io" "net/http" "os" "path/filepath" + "strings" "time" "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/log" "github.com/fatih/color" + "golang.org/x/mod/semver" ) const ( @@ -24,6 +26,10 @@ const ( defaultSkillsRepoRef = "v0.1.3" ) +// fetchFileFn is the function used to download individual skill files. +// It is a package-level var so tests can replace it with a mock. +var fetchFileFn = fetchSkillFile + func getSkillsRef(ctx context.Context) string { if ref := env.Get(ctx, "DATABRICKS_SKILLS_REF"); ref != "" { return ref @@ -40,44 +46,31 @@ type Manifest struct { // 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"` + Version string `json:"version"` + UpdatedAt string `json:"updated_at"` + Files []string `json:"files"` + Experimental bool `json:"experimental,omitempty"` + Description string `json:"description,omitempty"` + MinCLIVer string `json:"min_cli_version,omitempty"` +} + +// InstallOptions controls the behavior of InstallSkillsForAgents. +type InstallOptions struct { + IncludeExperimental bool + SpecificSkills []string // empty = all skills } // FetchManifest fetches the skills manifest from the skills repo. +// This is a convenience wrapper that uses the default GitHubManifestSource. func FetchManifest(ctx context.Context) (*Manifest, error) { + src := &GitHubManifestSource{} ref := getSkillsRef(ctx) - log.Infof(ctx, "Fetching skills manifest from %s/%s@%s", skillsRepoOwner, skillsRepoName, ref) - url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/manifest.json", - skillsRepoOwner, skillsRepoName, ref) - 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 + return src.FetchManifest(ctx, ref) } -func fetchSkillFile(ctx context.Context, skillName, filePath string) ([]byte, error) { +func fetchSkillFile(ctx context.Context, ref, skillName, filePath string) ([]byte, error) { url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s/%s/%s", - skillsRepoOwner, skillsRepoName, getSkillsRef(ctx), skillsRepoPath, skillName, filePath) + skillsRepoOwner, skillsRepoName, ref, skillsRepoPath, skillName, filePath) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { @@ -98,69 +91,174 @@ func fetchSkillFile(ctx context.Context, skillName, filePath string) ([]byte, er return io.ReadAll(resp.Body) } -// ListSkills fetches and prints available skills. -func ListSkills(ctx context.Context) error { - manifest, err := FetchManifest(ctx) +// InstallSkillsForAgents fetches the manifest and installs skills for the given agents. +// This is the core installation function. Callers are responsible for agent detection, +// prompting, and printing the "Installing..." header. +func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgents []*agents.Agent, opts InstallOptions) error { + latestTag, _, err := src.FetchLatestRelease(ctx) + if err != nil { + return fmt.Errorf("failed to fetch latest release: %w", err) + } + + manifest, err := src.FetchManifest(ctx, latestTag) if err != nil { return err } - cmdio.LogString(ctx, "Available skills:") - cmdio.LogString(ctx, "") + globalDir, err := GlobalSkillsDir(ctx) + if err != nil { + return err + } - for name, meta := range manifest.Skills { - cmdio.LogString(ctx, fmt.Sprintf(" %s (v%s)", name, meta.Version)) + // Load existing state for idempotency checks. + state, err := LoadState(globalDir) + if err != nil { + return fmt.Errorf("failed to load install state: %w", err) } - 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 -} + // Detect legacy installs (skills on disk but no state file). + if state == nil { + checkLegacyInstall(ctx, globalDir) + } -// InstallAllSkills fetches the manifest and installs all skills for detected agents. -func InstallAllSkills(ctx context.Context) error { - manifest, err := FetchManifest(ctx) + // Filter skills based on options, experimental flag, and CLI version. + targetSkills, err := resolveSkills(ctx, manifest.Skills, opts) if err != nil { return err } - detectedAgents := agents.DetectInstalled(ctx) - if len(detectedAgents) == 0 { - printNoAgentsDetected(ctx) - return nil - } - - printDetectedAgents(ctx, detectedAgents) + // Install each skill. + for name, meta := range targetSkills { + // Idempotency: skip if same version is already installed and on disk. + if state != nil && state.Skills[name] == meta.Version { + skillDir := filepath.Join(globalDir, name) + if _, statErr := os.Stat(skillDir); statErr == nil { + log.Debugf(ctx, "%s v%s already installed, skipping", name, meta.Version) + continue + } + } - for name, meta := range manifest.Skills { - if err := installSkillForAgents(ctx, name, meta.Files, detectedAgents); err != nil { + if err := installSkillForAgents(ctx, latestTag, name, meta.Files, targetAgents, globalDir); err != nil { return err } } + + // Save state. Merge into existing state so skills from previous installs + // (e.g., experimental skills from a prior run) are preserved. + existingState, _ := LoadState(globalDir) + var newState *InstallState + if existingState != nil { + newState = existingState + newState.Release = latestTag + newState.LastUpdated = time.Now() + newState.IncludeExperimental = opts.IncludeExperimental + for name, meta := range targetSkills { + newState.Skills[name] = meta.Version + } + } else { + newState = &InstallState{ + SchemaVersion: 1, + IncludeExperimental: opts.IncludeExperimental, + Release: latestTag, + LastUpdated: time.Now(), + Skills: make(map[string]string, len(targetSkills)), + } + for name, meta := range targetSkills { + newState.Skills[name] = meta.Version + } + } + if err := SaveState(globalDir, newState); err != nil { + return err + } + + tag := strings.TrimPrefix(latestTag, "v") + noun := "skills" + if len(targetSkills) == 1 { + noun = "skill" + } + cmdio.LogString(ctx, fmt.Sprintf("Installed %d %s (v%s).", len(targetSkills), noun, tag)) 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 +// resolveSkills filters the manifest skills based on the install options, +// experimental flag, and CLI version constraints. +func resolveSkills(ctx context.Context, skills map[string]SkillMeta, opts InstallOptions) (map[string]SkillMeta, error) { + isSpecific := len(opts.SpecificSkills) > 0 + cliVersion := build.GetInfo().Version + isDev := strings.HasPrefix(cliVersion, build.DefaultSemver) + + // Start with all skills or only the requested ones. + var candidates map[string]SkillMeta + if isSpecific { + candidates = make(map[string]SkillMeta, len(opts.SpecificSkills)) + for _, name := range opts.SpecificSkills { + meta, ok := skills[name] + if !ok { + return nil, fmt.Errorf("skill %q not found", name) + } + candidates[name] = meta + } + } else { + candidates = skills + } + + result := make(map[string]SkillMeta, len(candidates)) + for name, meta := range candidates { + if meta.Experimental && !opts.IncludeExperimental { + if isSpecific { + return nil, fmt.Errorf("skill %q is experimental; use --include-experimental to install", name) + } + log.Debugf(ctx, "Skipping experimental skill %s", name) + continue + } + + if meta.MinCLIVer != "" && !isDev && semver.Compare("v"+cliVersion, "v"+meta.MinCLIVer) < 0 { + if isSpecific { + return nil, fmt.Errorf("skill %q requires CLI version %s (running %s)", name, meta.MinCLIVer, cliVersion) + } + log.Warnf(ctx, "Skipping %s: requires CLI version %s (running %s)", name, meta.MinCLIVer, cliVersion) + continue + } + + result[name] = meta } + return result, nil +} - if _, ok := manifest.Skills[skillName]; !ok { - return fmt.Errorf("skill %q not found", skillName) +// InstallAllSkills fetches the manifest and installs all skills for detected agents. +// The signature is func(context.Context) error to satisfy the callback in cmd/apps/init.go. +func InstallAllSkills(ctx context.Context) error { + installed := agents.DetectInstalled(ctx) + if len(installed) == 0 { + printNoAgentsDetected(ctx) + return nil } - detectedAgents := agents.DetectInstalled(ctx) - if len(detectedAgents) == 0 { + PrintInstallingFor(ctx, installed) + src := &GitHubManifestSource{} + return InstallSkillsForAgents(ctx, src, installed, InstallOptions{}) +} + +// InstallSkill installs a single skill by name for all detected agents. +func InstallSkill(ctx context.Context, skillName string) error { + installed := agents.DetectInstalled(ctx) + if len(installed) == 0 { printNoAgentsDetected(ctx) return nil } - printDetectedAgents(ctx, detectedAgents) + PrintInstallingFor(ctx, installed) + src := &GitHubManifestSource{} + return InstallSkillsForAgents(ctx, src, installed, InstallOptions{SpecificSkills: []string{skillName}}) +} - return installSkillForAgents(ctx, skillName, manifest.Skills[skillName].Files, detectedAgents) +// PrintInstallingFor prints the "Installing..." header with agent names. +func PrintInstallingFor(ctx context.Context, targetAgents []*agents.Agent) { + names := make([]string, len(targetAgents)) + for i, a := range targetAgents { + names[i] = a.DisplayName + } + cmdio.LogString(ctx, fmt.Sprintf("Installing Databricks AI skills for %s...", strings.Join(names, ", "))) } func printNoAgentsDetected(ctx context.Context) { @@ -170,61 +268,73 @@ func printNoAgentsDetected(ctx context.Context) { 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) +// checkLegacyInstall prints a message if skills exist on disk but no state file was found. +func checkLegacyInstall(ctx context.Context, globalDir string) { + if hasSkillsOnDisk(globalDir) { + cmdio.LogString(ctx, "Found skills installed before state tracking was added. Run 'databricks experimental aitools install' to refresh.") + return + } + homeDir, err := env.UserHomeDir(ctx) + if err != nil { + return + } + legacyDir := filepath.Join(homeDir, ".databricks", "agent-skills") + if hasSkillsOnDisk(legacyDir) { + cmdio.LogString(ctx, "Found skills installed before state tracking was added. Run 'databricks experimental aitools install' to refresh.") } - cmdio.LogString(ctx, "") } -func installSkillForAgents(ctx context.Context, skillName string, files []string, detectedAgents []*agents.Agent) error { - homeDir, err := env.UserHomeDir(ctx) +// hasSkillsOnDisk checks if a directory contains subdirectories starting with "databricks". +func hasSkillsOnDisk(dir string) bool { + entries, err := os.ReadDir(dir) if err != nil { - return fmt.Errorf("failed to get home directory: %w", err) + return false } + for _, e := range entries { + if e.IsDir() && strings.HasPrefix(e.Name(), "databricks") { + return true + } + } + return false +} - // Always install to canonical location first. - canonicalDir := filepath.Join(homeDir, agents.CanonicalSkillsDir, skillName) - if err := installSkillToDir(ctx, skillName, canonicalDir, files); err != nil { +func installSkillForAgents(ctx context.Context, ref, skillName string, files []string, detectedAgents []*agents.Agent, globalDir string) error { + canonicalDir := filepath.Join(globalDir, skillName) + if err := installSkillToDir(ctx, ref, skillName, canonicalDir, files); err != nil { return err } useSymlinks := len(detectedAgents) > 1 - // install/symlink to each agent for _, agent := range detectedAgents { agentSkillDir, err := agent.SkillsDir(ctx) if err != nil { - cmdio.LogString(ctx, color.YellowString("⊘ Skipped %s: %v", agent.DisplayName, err)) + log.Warnf(ctx, "Skipped %s: %v", agent.DisplayName, err) continue } 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)) + log.Warnf(ctx, "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) - 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)) + log.Debugf(ctx, "Symlink failed for %s, copying instead: %v", agent.DisplayName, err) + if err := installSkillToDir(ctx, ref, skillName, destDir, files); err != nil { + log.Warnf(ctx, "Failed to install for %s: %v", agent.DisplayName, err) continue } } - cmdio.LogString(ctx, color.GreenString("✓ Installed %q for %s (symlinked)", skillName, agent.DisplayName)) + log.Debugf(ctx, "Installed %q for %s (symlinked)", skillName, agent.DisplayName) } else { - // 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)) + if err := installSkillToDir(ctx, ref, skillName, destDir, files); err != nil { + log.Warnf(ctx, "Failed to install for %s: %v", agent.DisplayName, err) continue } - cmdio.LogString(ctx, color.GreenString("✓ Installed %q for %s", skillName, agent.DisplayName)) + log.Debugf(ctx, "Installed %q for %s", skillName, agent.DisplayName) } } @@ -260,11 +370,11 @@ func backupThirdPartySkill(ctx context.Context, destDir, canonicalDir, skillName 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)) + log.Debugf(ctx, "Existing %q for %s moved to %s", skillName, agentName, backupDest) return nil } -func installSkillToDir(ctx context.Context, skillName, destDir string, files []string) error { +func installSkillToDir(ctx context.Context, ref, 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) @@ -274,20 +384,19 @@ func installSkillToDir(ctx context.Context, skillName, destDir string, files []s return fmt.Errorf("failed to create directory: %w", err) } - // download all files for _, file := range files { - content, err := fetchSkillFile(ctx, skillName, file) + content, err := fetchFileFn(ctx, ref, 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) } + log.Debugf(ctx, "Downloading %s/%s", skillName, file) if err := os.WriteFile(destPath, content, 0o644); err != nil { return fmt.Errorf("failed to write %s: %w", file, err) } diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go index 5caa1e3b57..e06910086a 100644 --- a/experimental/aitools/lib/installer/installer_test.go +++ b/experimental/aitools/lib/installer/installer_test.go @@ -1,15 +1,94 @@ package installer import ( + "bytes" + "context" + "log/slog" "os" "path/filepath" "testing" + "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// mockManifestSource is a test double for ManifestSource. +type mockManifestSource struct { + manifest *Manifest + release string + authoritative bool + fetchErr error +} + +func (m *mockManifestSource) FetchManifest(_ context.Context, _ string) (*Manifest, error) { + if m.fetchErr != nil { + return nil, m.fetchErr + } + return m.manifest, nil +} + +func (m *mockManifestSource) FetchLatestRelease(_ context.Context) (string, bool, error) { + return m.release, m.authoritative, nil +} + +func testManifest() *Manifest { + return &Manifest{ + Version: "1", + UpdatedAt: "2024-01-01", + Skills: map[string]SkillMeta{ + "databricks-sql": { + Version: "0.1.0", + Files: []string{"SKILL.md"}, + }, + "databricks-jobs": { + Version: "0.1.0", + Files: []string{"SKILL.md"}, + }, + }, + } +} + +func setupFetchMock(t *testing.T) { + t.Helper() + orig := fetchFileFn + t.Cleanup(func() { fetchFileFn = orig }) + fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) { + return []byte("# " + skillName + "/" + filePath), nil + } +} + +func testAgent(tmpHome string) *agents.Agent { + return &agents.Agent{ + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func(_ context.Context) (string, error) { + return filepath.Join(tmpHome, ".test-agent"), nil + }, + } +} + +func setupTestHome(t *testing.T) string { + t.Helper() + tmp := t.TempDir() + t.Setenv("HOME", tmp) + // Create agent config dir so the agent is "detected". + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".test-agent"), 0o755)) + return tmp +} + +func setBuildVersion(t *testing.T, version string) { + t.Helper() + orig := build.GetInfo().Version + build.SetBuildVersion(version) + t.Cleanup(func() { build.SetBuildVersion(orig) }) +} + +// --- Backup tests (unchanged from PR 1) --- + func TestBackupThirdPartySkillDestDoesNotExist(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) destDir := filepath.Join(t.TempDir(), "nonexistent") @@ -109,3 +188,301 @@ func TestBackupThirdPartySkillRegularFile(t *testing.T) { _, err = os.Stat(destDir) assert.True(t, os.IsNotExist(err)) } + +// --- InstallSkillsForAgents tests --- + +func TestInstallSkillsForAgentsWritesState(t *testing.T) { + tmp := setupTestHome(t) + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + setupFetchMock(t) + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) + require.NoError(t, err) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + require.NotNil(t, state) + assert.Equal(t, 1, state.SchemaVersion) + assert.Equal(t, "v0.1.0", state.Release) + assert.Len(t, state.Skills, 2) + assert.Equal(t, "0.1.0", state.Skills["databricks-sql"]) + assert.Equal(t, "0.1.0", state.Skills["databricks-jobs"]) + + assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.0).") +} + +func TestInstallSkillForSingleWritesState(t *testing.T) { + tmp := setupTestHome(t) + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + setupFetchMock(t) + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{ + SpecificSkills: []string{"databricks-sql"}, + }) + require.NoError(t, err) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + require.NotNil(t, state) + assert.Len(t, state.Skills, 1) + assert.Equal(t, "0.1.0", state.Skills["databricks-sql"]) + + assert.Contains(t, stderr.String(), "Installed 1 skill (v0.1.0).") +} + +func TestInstallSkillsSpecificNotFound(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{ + SpecificSkills: []string{"nonexistent"}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), `skill "nonexistent" not found`) +} + +func TestExperimentalSkillsSkippedByDefault(t *testing.T) { + tmp := setupTestHome(t) + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + setupFetchMock(t) + + manifest := testManifest() + manifest.Skills["databricks-experimental"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + Experimental: true, + } + + src := &mockManifestSource{manifest: manifest, release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) + require.NoError(t, err) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + // Only non-experimental skills should be installed. + assert.Len(t, state.Skills, 2) + assert.NotContains(t, state.Skills, "databricks-experimental") + + assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.0).") +} + +func TestExperimentalSkillsIncludedWithFlag(t *testing.T) { + tmp := setupTestHome(t) + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + setupFetchMock(t) + + manifest := testManifest() + manifest.Skills["databricks-experimental"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + Experimental: true, + } + + src := &mockManifestSource{manifest: manifest, release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{ + IncludeExperimental: true, + }) + require.NoError(t, err) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.Len(t, state.Skills, 3) + assert.Contains(t, state.Skills, "databricks-experimental") + assert.True(t, state.IncludeExperimental) + + assert.Contains(t, stderr.String(), "Installed 3 skills (v0.1.0).") +} + +func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) { + tmp := setupTestHome(t) + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + setupFetchMock(t) + setBuildVersion(t, "0.200.0") + + // Capture log output to verify the warning. + var logBuf bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logBuf, &slog.HandlerOptions{Level: slog.LevelWarn})) + ctx = log.NewContext(ctx, logger) + + manifest := testManifest() + manifest.Skills["databricks-future"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + MinCLIVer: "0.300.0", + } + + src := &mockManifestSource{manifest: manifest, release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) + require.NoError(t, err) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + // The high-version skill should be skipped. + assert.Len(t, state.Skills, 2) + assert.NotContains(t, state.Skills, "databricks-future") + + assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.0).") + assert.Contains(t, logBuf.String(), "requires CLI version 0.300.0") +} + +func TestMinCLIVersionHardErrorForInstallSingle(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + setBuildVersion(t, "0.200.0") + + manifest := testManifest() + manifest.Skills["databricks-future"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + MinCLIVer: "0.300.0", + } + + src := &mockManifestSource{manifest: manifest, release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{ + SpecificSkills: []string{"databricks-future"}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "requires CLI version 0.300.0") + assert.Contains(t, err.Error(), "running 0.200.0") +} + +func TestIdempotentSecondInstallSkips(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + // First install. + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) + require.NoError(t, err) + + // Track fetch calls on second install. + fetchCalls := 0 + orig := fetchFileFn + t.Cleanup(func() { fetchFileFn = orig }) + fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) { + fetchCalls++ + return []byte("# " + skillName + "/" + filePath), nil + } + + // Second install with same version. + err = InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) + require.NoError(t, err) + + // No files should be fetched since everything is up to date. + assert.Equal(t, 0, fetchCalls) +} + +func TestIdempotentInstallUpdatesNewVersions(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + // First install. + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) + require.NoError(t, err) + + // Update manifest with a new version for one skill. + updatedManifest := testManifest() + updatedManifest.Skills["databricks-sql"] = SkillMeta{ + Version: "0.2.0", + Files: []string{"SKILL.md"}, + } + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + + // Track which skills are fetched. + var fetchedSkills []string + orig := fetchFileFn + t.Cleanup(func() { fetchFileFn = orig }) + fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) { + fetchedSkills = append(fetchedSkills, skillName) + return []byte("# " + skillName + "/" + filePath), nil + } + + // Second install with updated manifest. + err = InstallSkillsForAgents(ctx, src2, []*agents.Agent{agent}, InstallOptions{}) + require.NoError(t, err) + + // Both skills should be fetched because the release tag changed. + // (databricks-sql has a new version, databricks-jobs has the same version + // but state was from v0.1.0 release.) + assert.Contains(t, fetchedSkills, "databricks-sql") + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.Equal(t, "v0.2.0", state.Release) + assert.Equal(t, "0.2.0", state.Skills["databricks-sql"]) +} + +func TestLegacyDetectMessagePrinted(t *testing.T) { + tmp := setupTestHome(t) + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + setupFetchMock(t) + + // Create skills on disk at canonical location but no state file. + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + require.NoError(t, os.MkdirAll(filepath.Join(globalDir, "databricks-sql"), 0o755)) + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) + require.NoError(t, err) + + assert.Contains(t, stderr.String(), "Found skills installed before state tracking was added.") +} + +func TestLegacyDetectLegacyDir(t *testing.T) { + tmp := setupTestHome(t) + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + setupFetchMock(t) + + // Create skills in the legacy location. + legacyDir := filepath.Join(tmp, ".databricks", "agent-skills") + require.NoError(t, os.MkdirAll(filepath.Join(legacyDir, "databricks-sql"), 0o755)) + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) + require.NoError(t, err) + + assert.Contains(t, stderr.String(), "Found skills installed before state tracking was added.") +} + +func TestInstallAllSkillsSignaturePreserved(t *testing.T) { + // Compile-time check that InstallAllSkills matches the expected signature. + // cmd/apps/init.go passes this as func(context.Context) error. + callback := func(fn func(context.Context) error) { _ = fn } + callback(InstallAllSkills) +} diff --git a/experimental/aitools/lib/installer/source.go b/experimental/aitools/lib/installer/source.go new file mode 100644 index 0000000000..c375d108d4 --- /dev/null +++ b/experimental/aitools/lib/installer/source.go @@ -0,0 +1,109 @@ +package installer + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "time" + + "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" +) + +// ManifestSource abstracts how the skills manifest and release info are fetched. +type ManifestSource interface { + // FetchManifest fetches the skills manifest at the given ref. + FetchManifest(ctx context.Context, ref string) (*Manifest, error) + + // FetchLatestRelease returns the latest release tag and whether the result + // is authoritative. When authoritative is true, the tag came from a + // successful API call. When false, the tag is a fallback default (e.g., + // due to network failure). Callers should use this to decide whether + // to trust the result for staleness comparisons. + FetchLatestRelease(ctx context.Context) (tag string, authoritative bool, err error) +} + +// GitHubManifestSource fetches manifests and release info from GitHub. +type GitHubManifestSource struct{} + +// FetchManifest fetches the skills manifest from GitHub at the given ref. +func (s *GitHubManifestSource) FetchManifest(ctx context.Context, ref string) (*Manifest, error) { + log.Debugf(ctx, "Fetching skills manifest from %s/%s@%s", skillsRepoOwner, skillsRepoName, ref) + url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/manifest.json", + skillsRepoOwner, skillsRepoName, ref) + + 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 +} + +// FetchLatestRelease returns the latest release tag from GitHub. +// If DATABRICKS_SKILLS_REF is set, it is returned as authoritative. +// On any error (network, non-200, parse), falls back to defaultSkillsRepoRef +// with authoritative=false. +// +// The DATABRICKS_SKILLS_REF check is intentionally duplicated in getSkillsRef() +// because callers may use either the ManifestSource interface directly or the +// convenience FetchManifest wrapper. +func (s *GitHubManifestSource) FetchLatestRelease(ctx context.Context) (string, bool, error) { + if ref := env.Get(ctx, "DATABRICKS_SKILLS_REF"); ref != "" { + return ref, true, nil + } + + url := fmt.Sprintf("https://api.github.com/repos/%s/%s/releases/latest", + skillsRepoOwner, skillsRepoName) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + log.Debugf(ctx, "Failed to create release request, falling back to %s: %v", defaultSkillsRepoRef, err) + return defaultSkillsRepoRef, false, nil + } + + client := &http.Client{Timeout: 15 * time.Second} + resp, err := client.Do(req) + if err != nil { + log.Debugf(ctx, "Failed to fetch latest release, falling back to %s: %v", defaultSkillsRepoRef, err) + return defaultSkillsRepoRef, false, nil + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + log.Debugf(ctx, "Latest release returned HTTP %d, falling back to %s", resp.StatusCode, defaultSkillsRepoRef) + return defaultSkillsRepoRef, false, nil + } + + var release struct { + TagName string `json:"tag_name"` + } + if err := json.NewDecoder(resp.Body).Decode(&release); err != nil { + log.Debugf(ctx, "Failed to parse release response, falling back to %s: %v", defaultSkillsRepoRef, err) + return defaultSkillsRepoRef, false, nil + } + + if release.TagName == "" { + log.Debugf(ctx, "Empty tag_name in release response, falling back to %s", defaultSkillsRepoRef) + return defaultSkillsRepoRef, false, nil + } + + return release.TagName, true, nil +} diff --git a/experimental/aitools/lib/installer/state.go b/experimental/aitools/lib/installer/state.go new file mode 100644 index 0000000000..ec17db1f08 --- /dev/null +++ b/experimental/aitools/lib/installer/state.go @@ -0,0 +1,98 @@ +package installer + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/databricks/cli/libs/env" +) + +const stateFileName = ".state.json" + +// ErrNotImplemented indicates that a feature is not yet implemented. +var ErrNotImplemented = errors.New("project scope not yet implemented") + +// InstallState records the state of all installed skills in a scope directory. +type InstallState struct { + SchemaVersion int `json:"schema_version"` + IncludeExperimental bool `json:"include_experimental,omitempty"` + Release string `json:"release"` + LastUpdated time.Time `json:"last_updated"` + Skills map[string]string `json:"skills"` + Scope string `json:"scope,omitempty"` +} + +// LoadState reads install state from the given directory. +// Returns (nil, nil) when the state file does not exist. +func LoadState(dir string) (*InstallState, error) { + data, err := os.ReadFile(filepath.Join(dir, stateFileName)) + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("failed to read state file: %w", err) + } + + var state InstallState + if err := json.Unmarshal(data, &state); err != nil { + return nil, fmt.Errorf("failed to parse state file: %w", err) + } + return &state, nil +} + +// SaveState writes install state to the given directory atomically. +// Creates the directory if it does not exist. +func SaveState(dir string, state *InstallState) error { + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("failed to create state directory: %w", err) + } + + data, err := json.MarshalIndent(state, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal state: %w", err) + } + data = append(data, '\n') + + // Atomic write: write to temp file in the same directory, then rename. + tmp, err := os.CreateTemp(dir, ".state-*.tmp") + if err != nil { + return fmt.Errorf("failed to create temp file: %w", err) + } + tmpName := tmp.Name() + + if _, err := tmp.Write(data); err != nil { + tmp.Close() + os.Remove(tmpName) + return fmt.Errorf("failed to write temp file: %w", err) + } + if err := tmp.Close(); err != nil { + os.Remove(tmpName) + return fmt.Errorf("failed to close temp file: %w", err) + } + + if err := os.Rename(tmpName, filepath.Join(dir, stateFileName)); err != nil { + os.Remove(tmpName) + return fmt.Errorf("failed to rename state file: %w", err) + } + return nil +} + +// GlobalSkillsDir returns the path to the global skills directory (~/.databricks/aitools/skills/). +func GlobalSkillsDir(ctx context.Context) (string, error) { + home, err := env.UserHomeDir(ctx) + if err != nil { + return "", err + } + return filepath.Join(home, ".databricks", "aitools", "skills"), nil +} + +// ProjectSkillsDir returns the path to the project-scoped skills directory. +// Project scope is not yet implemented. +func ProjectSkillsDir(_ context.Context) (string, error) { + return "", ErrNotImplemented +} diff --git a/experimental/aitools/lib/installer/state_test.go b/experimental/aitools/lib/installer/state_test.go new file mode 100644 index 0000000000..ea459dfafe --- /dev/null +++ b/experimental/aitools/lib/installer/state_test.go @@ -0,0 +1,116 @@ +package installer + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/databricks/cli/libs/env" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoadStateNonexistentFile(t *testing.T) { + state, err := LoadState(t.TempDir()) + assert.NoError(t, err) + assert.Nil(t, state) +} + +func TestSaveAndLoadStateRoundtrip(t *testing.T) { + dir := t.TempDir() + original := &InstallState{ + SchemaVersion: 1, + Release: "v0.2.0", + LastUpdated: time.Date(2026, 3, 22, 10, 0, 0, 0, time.UTC), + Skills: map[string]string{ + "databricks": "1.0.0", + }, + } + + err := SaveState(dir, original) + require.NoError(t, err) + + loaded, err := LoadState(dir) + require.NoError(t, err) + assert.Equal(t, original, loaded) +} + +func TestSaveStateCreatesDirectory(t *testing.T) { + dir := filepath.Join(t.TempDir(), "nested", "path") + state := &InstallState{ + SchemaVersion: 1, + Release: "v0.1.0", + LastUpdated: time.Date(2026, 3, 22, 9, 0, 0, 0, time.UTC), + Skills: map[string]string{}, + } + + err := SaveState(dir, state) + require.NoError(t, err) + + // Verify file exists. + _, err = os.Stat(filepath.Join(dir, stateFileName)) + assert.NoError(t, err) +} + +func TestSaveStateTrailingNewline(t *testing.T) { + dir := t.TempDir() + state := &InstallState{ + SchemaVersion: 1, + Release: "v0.1.0", + LastUpdated: time.Date(2026, 3, 22, 9, 0, 0, 0, time.UTC), + Skills: map[string]string{}, + } + + err := SaveState(dir, state) + require.NoError(t, err) + + data, err := os.ReadFile(filepath.Join(dir, stateFileName)) + require.NoError(t, err) + assert.Equal(t, byte('\n'), data[len(data)-1]) +} + +func TestLoadStateCorruptJSON(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, stateFileName), []byte("{bad json"), 0o644)) + + state, err := LoadState(dir) + assert.Error(t, err) + assert.Nil(t, state) + assert.Contains(t, err.Error(), "failed to parse state file") +} + +func TestGlobalSkillsDir(t *testing.T) { + ctx := env.WithUserHomeDir(t.Context(), "/fake/home") + dir, err := GlobalSkillsDir(ctx) + require.NoError(t, err) + assert.Equal(t, filepath.Join("/fake/home", ".databricks", "aitools", "skills"), dir) +} + +func TestProjectSkillsDirNotImplemented(t *testing.T) { + dir, err := ProjectSkillsDir(t.Context()) + assert.ErrorIs(t, err, ErrNotImplemented) + assert.Empty(t, dir) +} + +func TestSaveAndLoadStateWithOptionalFields(t *testing.T) { + dir := t.TempDir() + original := &InstallState{ + SchemaVersion: 1, + IncludeExperimental: true, + Release: "v0.3.0", + LastUpdated: time.Date(2026, 3, 22, 12, 30, 0, 0, time.UTC), + Skills: map[string]string{ + "databricks": "1.0.0", + "sql-tools": "0.2.0", + }, + Scope: "project", + } + + err := SaveState(dir, original) + require.NoError(t, err) + + loaded, err := LoadState(dir) + require.NoError(t, err) + assert.Equal(t, original, loaded) +} diff --git a/experimental/aitools/lib/installer/uninstall.go b/experimental/aitools/lib/installer/uninstall.go new file mode 100644 index 0000000000..e0d9a63083 --- /dev/null +++ b/experimental/aitools/lib/installer/uninstall.go @@ -0,0 +1,189 @@ +package installer + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/log" +) + +// UninstallOptions controls the behavior of UninstallSkillsOpts. +type UninstallOptions struct { + Skills []string // empty = all +} + +// UninstallSkills removes all installed skills, their symlinks, and the state file. +func UninstallSkills(ctx context.Context) error { + return UninstallSkillsOpts(ctx, UninstallOptions{}) +} + +// UninstallSkillsOpts removes installed skills based on options. +// When opts.Skills is empty, all skills are removed (same as UninstallSkills). +// When opts.Skills is non-empty, only the named skills are removed. +func UninstallSkillsOpts(ctx context.Context, opts UninstallOptions) error { + globalDir, err := GlobalSkillsDir(ctx) + if err != nil { + return err + } + + state, err := LoadState(globalDir) + if err != nil { + return fmt.Errorf("failed to load install state: %w", err) + } + + if state == nil { + if hasLegacyInstall(ctx, globalDir) { + return errors.New("found skills from a previous install without state tracking; run 'databricks experimental aitools install' first, then uninstall") + } + return errors.New("no skills installed") + } + + // Determine which skills to remove. + var toRemove []string + if len(opts.Skills) > 0 { + for _, name := range opts.Skills { + if _, ok := state.Skills[name]; !ok { + return fmt.Errorf("skill %q is not installed", name) + } + toRemove = append(toRemove, name) + } + } else { + for name := range state.Skills { + toRemove = append(toRemove, name) + } + } + + removeAll := len(toRemove) == len(state.Skills) + + // Remove skill directories and symlinks for each skill. + for _, name := range toRemove { + canonicalDir := filepath.Join(globalDir, name) + removeSymlinksFromAgents(ctx, name, canonicalDir) + if err := os.RemoveAll(canonicalDir); err != nil { + log.Warnf(ctx, "Failed to remove %s: %v", canonicalDir, err) + } + delete(state.Skills, name) + } + + if removeAll { + // Clean up orphaned symlinks and delete state file. + cleanOrphanedSymlinks(ctx, globalDir) + stateFile := filepath.Join(globalDir, stateFileName) + if err := os.Remove(stateFile); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to remove state file: %w", err) + } + } else { + // Update state to reflect remaining skills. + if err := SaveState(globalDir, state); err != nil { + return err + } + } + + noun := "skills" + if len(toRemove) == 1 { + noun = "skill" + } + cmdio.LogString(ctx, fmt.Sprintf("Uninstalled %d %s.", len(toRemove), noun)) + return nil +} + +// removeSymlinksFromAgents removes a skill's symlink from all agent directories +// in the registry, but only if the entry is a symlink pointing into canonicalDir. +// Non-symlink directories are left untouched to avoid deleting user-managed content. +func removeSymlinksFromAgents(ctx context.Context, skillName, canonicalDir string) { + for i := range agents.Registry { + agent := &agents.Registry[i] + skillsDir, err := agent.SkillsDir(ctx) + if err != nil { + continue + } + + destDir := filepath.Join(skillsDir, skillName) + + // Use Lstat to detect symlinks (Stat follows them). + fi, err := os.Lstat(destDir) + if os.IsNotExist(err) { + continue + } + if err != nil { + log.Warnf(ctx, "Failed to stat %s for %s: %v", destDir, agent.DisplayName, err) + continue + } + + if fi.Mode()&os.ModeSymlink == 0 { + log.Debugf(ctx, "Skipping non-symlink %s for %s", destDir, agent.DisplayName) + continue + } + + target, err := os.Readlink(destDir) + if err != nil { + log.Warnf(ctx, "Failed to read symlink %s: %v", destDir, err) + continue + } + + // Only remove if the symlink points into our canonical dir. + if !strings.HasPrefix(target, canonicalDir+string(os.PathSeparator)) && target != canonicalDir { + log.Debugf(ctx, "Skipping symlink %s (points to %s, not %s)", destDir, target, canonicalDir) + continue + } + + if err := os.Remove(destDir); err != nil { + log.Warnf(ctx, "Failed to remove symlink %s: %v", destDir, err) + } else { + log.Debugf(ctx, "Removed %q from %s", skillName, agent.DisplayName) + } + } +} + +// cleanOrphanedSymlinks scans all agent skill directories for symlinks pointing +// into globalDir that are not tracked in state, and removes them. +func cleanOrphanedSymlinks(ctx context.Context, globalDir string) { + for i := range agents.Registry { + agent := &agents.Registry[i] + skillsDir, err := agent.SkillsDir(ctx) + if err != nil { + continue + } + + entries, err := os.ReadDir(skillsDir) + if err != nil { + continue + } + + for _, entry := range entries { + entryPath := filepath.Join(skillsDir, entry.Name()) + + fi, err := os.Lstat(entryPath) + if err != nil { + continue + } + + if fi.Mode()&os.ModeSymlink == 0 { + continue + } + + target, err := os.Readlink(entryPath) + if err != nil { + continue + } + + // Check if the symlink points into our global skills dir. + if !strings.HasPrefix(target, globalDir+string(os.PathSeparator)) && target != globalDir { + continue + } + + // This symlink points into our managed dir. Remove it. + if err := os.Remove(entryPath); err != nil { + log.Warnf(ctx, "Failed to remove orphaned symlink %s: %v", entryPath, err) + } else { + log.Debugf(ctx, "Removed orphaned symlink %s -> %s", entryPath, target) + } + } + } +} diff --git a/experimental/aitools/lib/installer/uninstall_test.go b/experimental/aitools/lib/installer/uninstall_test.go new file mode 100644 index 0000000000..fe52daf37a --- /dev/null +++ b/experimental/aitools/lib/installer/uninstall_test.go @@ -0,0 +1,306 @@ +package installer + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/libs/cmdio" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func installTestSkills(t *testing.T, tmp string) string { + t.Helper() + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + return filepath.Join(tmp, ".databricks", "aitools", "skills") +} + +func TestUninstallRemovesSkillDirectories(t *testing.T) { + tmp := setupTestHome(t) + globalDir := installTestSkills(t, tmp) + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + + err := UninstallSkills(ctx) + require.NoError(t, err) + + // Skill directories should be gone. + _, err = os.Stat(filepath.Join(globalDir, "databricks-sql")) + assert.True(t, os.IsNotExist(err)) + _, err = os.Stat(filepath.Join(globalDir, "databricks-jobs")) + assert.True(t, os.IsNotExist(err)) + + assert.Contains(t, stderr.String(), "Uninstalled 2 skills.") +} + +func TestUninstallRemovesSymlinks(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + // Use two registry-based agents so uninstall can find them. + // Create config dirs for claude-code and cursor (both in agents.Registry). + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".claude"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".cursor"), 0o755)) + + claudeAgent := &agents.Agent{ + Name: "claude-code", + DisplayName: "Claude Code", + ConfigDir: func(_ context.Context) (string, error) { + return filepath.Join(tmp, ".claude"), nil + }, + } + cursorAgent := &agents.Agent{ + Name: "cursor", + DisplayName: "Cursor", + ConfigDir: func(_ context.Context) (string, error) { + return filepath.Join(tmp, ".cursor"), nil + }, + } + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{claudeAgent, cursorAgent}, InstallOptions{})) + + ctx2, _ := cmdio.NewTestContextWithStderr(t.Context()) + err := UninstallSkills(ctx2) + require.NoError(t, err) + + // Check that agent skill directories are cleaned up. + // These agents are in agents.Registry so removeSymlinksFromAgents finds them. + for _, agentDir := range []string{".claude", ".cursor"} { + sqlLink := filepath.Join(tmp, agentDir, "skills", "databricks-sql") + _, err := os.Lstat(sqlLink) + assert.True(t, os.IsNotExist(err), "symlink should be removed from %s", agentDir) + + jobsLink := filepath.Join(tmp, agentDir, "skills", "databricks-jobs") + _, err = os.Lstat(jobsLink) + assert.True(t, os.IsNotExist(err), "symlink should be removed from %s", agentDir) + } +} + +func TestUninstallCleansOrphanedSymlinks(t *testing.T) { + tmp := setupTestHome(t) + globalDir := installTestSkills(t, tmp) + + // Create an orphaned symlink in a registry agent's dir that points into + // globalDir but is not tracked in state. + // .claude is in agents.Registry so cleanOrphanedSymlinks will scan it. + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".claude"), 0o755)) + agentSkillsDir := filepath.Join(tmp, ".claude", "skills") + require.NoError(t, os.MkdirAll(agentSkillsDir, 0o755)) + + orphanTarget := filepath.Join(globalDir, "databricks-orphan") + require.NoError(t, os.MkdirAll(orphanTarget, 0o755)) + orphanLink := filepath.Join(agentSkillsDir, "databricks-orphan") + require.NoError(t, os.Symlink(orphanTarget, orphanLink)) + + ctx, _ := cmdio.NewTestContextWithStderr(t.Context()) + err := UninstallSkills(ctx) + require.NoError(t, err) + + // Orphaned symlink should be removed. + _, err = os.Lstat(orphanLink) + assert.True(t, os.IsNotExist(err)) +} + +func TestUninstallDeletesStateFile(t *testing.T) { + tmp := setupTestHome(t) + globalDir := installTestSkills(t, tmp) + + // Verify state file exists before uninstall. + _, err := os.Stat(filepath.Join(globalDir, ".state.json")) + require.NoError(t, err) + + ctx := cmdio.MockDiscard(t.Context()) + err = UninstallSkills(ctx) + require.NoError(t, err) + + // State file should be gone. + _, err = os.Stat(filepath.Join(globalDir, ".state.json")) + assert.True(t, os.IsNotExist(err)) +} + +func TestUninstallNoStateReturnsError(t *testing.T) { + setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + + err := UninstallSkills(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "no skills installed") +} + +func TestUninstallHandlesMissingDirectories(t *testing.T) { + tmp := setupTestHome(t) + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + + // Create state manually but without actual skill directories on disk. + state := &InstallState{ + SchemaVersion: 1, + Release: "v0.1.0", + LastUpdated: time.Now(), + Skills: map[string]string{ + "databricks-sql": "0.1.0", + "databricks-jobs": "0.1.0", + }, + } + require.NoError(t, SaveState(globalDir, state)) + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + err := UninstallSkills(ctx) + require.NoError(t, err) + assert.Contains(t, stderr.String(), "Uninstalled 2 skills.") +} + +func TestUninstallHandlesBrokenSymlinksToCanonicalDir(t *testing.T) { + tmp := setupTestHome(t) + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + + // Create state with one skill. + state := &InstallState{ + SchemaVersion: 1, + Release: "v0.1.0", + LastUpdated: time.Now(), + Skills: map[string]string{ + "databricks-sql": "0.1.0", + }, + } + require.NoError(t, SaveState(globalDir, state)) + + // Create a symlink pointing to the canonical dir (which doesn't exist on disk). + canonicalTarget := filepath.Join(globalDir, "databricks-sql") + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".claude"), 0o755)) + agentSkillsDir := filepath.Join(tmp, ".claude", "skills") + require.NoError(t, os.MkdirAll(agentSkillsDir, 0o755)) + link := filepath.Join(agentSkillsDir, "databricks-sql") + require.NoError(t, os.Symlink(canonicalTarget, link)) + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + err := UninstallSkills(ctx) + require.NoError(t, err) + + // Symlink pointing to canonical dir should be removed. + _, err = os.Lstat(link) + assert.True(t, os.IsNotExist(err)) + assert.Contains(t, stderr.String(), "Uninstalled 1 skill.") +} + +func TestUninstallLeavesNonCanonicalSymlinks(t *testing.T) { + tmp := setupTestHome(t) + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + + state := &InstallState{ + SchemaVersion: 1, + Release: "v0.1.0", + LastUpdated: time.Now(), + Skills: map[string]string{ + "databricks-sql": "0.1.0", + }, + } + require.NoError(t, SaveState(globalDir, state)) + + // Create a symlink in an agent dir pointing somewhere other than canonical dir. + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".claude"), 0o755)) + agentSkillsDir := filepath.Join(tmp, ".claude", "skills") + require.NoError(t, os.MkdirAll(agentSkillsDir, 0o755)) + externalLink := filepath.Join(agentSkillsDir, "databricks-sql") + require.NoError(t, os.Symlink("/some/other/place", externalLink)) + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + err := UninstallSkills(ctx) + require.NoError(t, err) + + // Symlink pointing outside canonical dir should be preserved. + _, err = os.Lstat(externalLink) + assert.NoError(t, err, "symlink to external path should not be removed") + assert.Contains(t, stderr.String(), "Uninstalled 1 skill.") +} + +func TestUninstallLeavesNonSymlinkDirectories(t *testing.T) { + tmp := setupTestHome(t) + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + + state := &InstallState{ + SchemaVersion: 1, + Release: "v0.1.0", + LastUpdated: time.Now(), + Skills: map[string]string{ + "databricks-sql": "0.1.0", + }, + } + require.NoError(t, SaveState(globalDir, state)) + + // Create a regular directory (not symlink) in agent skills dir. + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".claude"), 0o755)) + agentSkillsDir := filepath.Join(tmp, ".claude", "skills") + regularDir := filepath.Join(agentSkillsDir, "databricks-sql") + require.NoError(t, os.MkdirAll(regularDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(regularDir, "custom.md"), []byte("custom"), 0o644)) + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + err := UninstallSkills(ctx) + require.NoError(t, err) + + // Regular directory should be preserved (not a symlink to canonical dir). + _, err = os.Stat(regularDir) + assert.NoError(t, err, "regular directory should not be removed") + assert.Contains(t, stderr.String(), "Uninstalled 1 skill.") +} + +func TestUninstallSelectiveRemovesOnlyNamedSkills(t *testing.T) { + tmp := setupTestHome(t) + globalDir := installTestSkills(t, tmp) + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + err := UninstallSkillsOpts(ctx, UninstallOptions{Skills: []string{"databricks-sql"}}) + require.NoError(t, err) + + // databricks-sql should be gone. + _, err = os.Stat(filepath.Join(globalDir, "databricks-sql")) + assert.True(t, os.IsNotExist(err)) + + // databricks-jobs should still exist. + _, err = os.Stat(filepath.Join(globalDir, "databricks-jobs")) + assert.NoError(t, err) + + // State should still exist with remaining skill. + state, err := LoadState(globalDir) + require.NoError(t, err) + require.NotNil(t, state) + assert.Len(t, state.Skills, 1) + assert.Contains(t, state.Skills, "databricks-jobs") + + assert.Contains(t, stderr.String(), "Uninstalled 1 skill.") +} + +func TestUninstallSelectiveUnknownSkillErrors(t *testing.T) { + tmp := setupTestHome(t) + installTestSkills(t, tmp) + + ctx := cmdio.MockDiscard(t.Context()) + err := UninstallSkillsOpts(ctx, UninstallOptions{Skills: []string{"nonexistent"}}) + require.Error(t, err) + assert.Contains(t, err.Error(), "not installed") +} + +func TestUninstallSelectiveAllRemovesStateFile(t *testing.T) { + tmp := setupTestHome(t) + globalDir := installTestSkills(t, tmp) + + ctx, _ := cmdio.NewTestContextWithStderr(t.Context()) + err := UninstallSkillsOpts(ctx, UninstallOptions{Skills: []string{"databricks-sql", "databricks-jobs"}}) + require.NoError(t, err) + + // State file should be gone since all skills were removed. + _, err = os.Stat(filepath.Join(globalDir, ".state.json")) + assert.True(t, os.IsNotExist(err)) +} diff --git a/experimental/aitools/lib/installer/update.go b/experimental/aitools/lib/installer/update.go new file mode 100644 index 0000000000..6ba461cd10 --- /dev/null +++ b/experimental/aitools/lib/installer/update.go @@ -0,0 +1,257 @@ +package installer + +import ( + "context" + "errors" + "fmt" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/internal/build" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" + "golang.org/x/mod/semver" +) + +// UpdateOptions controls the behavior of UpdateSkills. +type UpdateOptions struct { + Force bool + NoNew bool + Check bool // dry run: show what would change without downloading + Skills []string // empty = all installed +} + +// UpdateResult describes what UpdateSkills did (or would do in check mode). +type UpdateResult struct { + Updated []SkillUpdate // skills that were updated + Added []SkillUpdate // new skills added (when NoNew is false) + Unchanged []string // skills at current version + Skipped []string // skills skipped (experimental, version constraint) +} + +// SkillUpdate describes a single skill version change. +type SkillUpdate struct { + Name string + OldVersion string + NewVersion string +} + +// UpdateSkills updates installed skills to the latest release. +func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agents.Agent, opts UpdateOptions) (*UpdateResult, error) { + globalDir, err := GlobalSkillsDir(ctx) + if err != nil { + return nil, err + } + + state, err := LoadState(globalDir) + if err != nil { + return nil, fmt.Errorf("failed to load install state: %w", err) + } + + if state == nil { + if hasLegacyInstall(ctx, globalDir) { + return nil, errors.New("found skills from a previous install without state tracking; run 'databricks experimental aitools install' to refresh before updating") + } + return nil, errors.New("no skills installed. Run 'databricks experimental aitools install' to install") + } + + latestTag, authoritative, err := src.FetchLatestRelease(ctx) + if err != nil { + return nil, fmt.Errorf("failed to fetch latest release: %w", err) + } + + if !authoritative && !opts.Force { + cmdio.LogString(ctx, "Could not check for updates (offline?). Use --force to update anyway.") + return &UpdateResult{Unchanged: sortedKeys(state.Skills)}, nil + } + + if state.Release == latestTag && !opts.Force { + cmdio.LogString(ctx, "Already up to date.") + return &UpdateResult{Unchanged: sortedKeys(state.Skills)}, nil + } + + manifest, err := src.FetchManifest(ctx, latestTag) + if err != nil { + if opts.Check { + log.Warnf(ctx, "Could not fetch manifest: %v", err) + return &UpdateResult{}, nil + } + return nil, err + } + + // Determine the skill set to consider. + skillSet := buildUpdateSkillSet(state, manifest, opts) + + result := &UpdateResult{} + + cliVersion := build.GetInfo().Version + isDev := strings.HasPrefix(cliVersion, build.DefaultSemver) + + // Sort skill names for deterministic output. + names := sortedKeys(skillSet) + + for _, name := range names { + meta, inManifest := manifest.Skills[name] + oldVersion := state.Skills[name] + + if !inManifest { + _, wasInstalled := state.Skills[name] + if wasInstalled { + log.Warnf(ctx, "Warning: %q not found in manifest %s (keeping installed version).", name, latestTag) + } + result.Unchanged = append(result.Unchanged, name) + continue + } + + // Filter experimental skills unless state opted in. + if meta.Experimental && !state.IncludeExperimental { + log.Debugf(ctx, "Skipping experimental skill %s", name) + result.Skipped = append(result.Skipped, name) + continue + } + + // Filter skills requiring a newer CLI version. + if meta.MinCLIVer != "" && !isDev && semver.Compare("v"+cliVersion, "v"+meta.MinCLIVer) < 0 { + log.Warnf(ctx, "Skipping %s: requires CLI version %s (running %s)", name, meta.MinCLIVer, cliVersion) + result.Skipped = append(result.Skipped, name) + continue + } + + // Check if this is a new skill (not in state). + _, wasInstalled := state.Skills[name] + + if meta.Version == oldVersion && !opts.Force { + result.Unchanged = append(result.Unchanged, name) + continue + } + + update := SkillUpdate{ + Name: name, + OldVersion: oldVersion, + NewVersion: meta.Version, + } + + if !wasInstalled { + result.Added = append(result.Added, update) + } else { + result.Updated = append(result.Updated, update) + } + } + + if opts.Check { + return result, nil + } + + // Download and install updated/added skills. + allChanges := append(result.Updated, result.Added...) + for _, change := range allChanges { + meta := manifest.Skills[change.Name] + if err := installSkillForAgents(ctx, latestTag, change.Name, meta.Files, targetAgents, globalDir); err != nil { + return nil, err + } + } + + // Update state. + state.Release = latestTag + state.LastUpdated = time.Now() + for _, change := range allChanges { + state.Skills[change.Name] = change.NewVersion + } + if err := SaveState(globalDir, state); err != nil { + return nil, err + } + + return result, nil +} + +// buildUpdateSkillSet determines which skills to consider for update. +func buildUpdateSkillSet(state *InstallState, manifest *Manifest, opts UpdateOptions) map[string]bool { + skillSet := make(map[string]bool) + + if len(opts.Skills) > 0 { + // Only named skills. + for _, name := range opts.Skills { + skillSet[name] = true + } + return skillSet + } + + // All installed skills. + for name := range state.Skills { + skillSet[name] = true + } + + // Auto-add new skills from manifest (unless --no-new). + if !opts.NoNew { + for name := range manifest.Skills { + skillSet[name] = true + } + } + + return skillSet +} + +// hasLegacyInstall checks both canonical and legacy dirs for skills on disk without state. +func hasLegacyInstall(ctx context.Context, globalDir string) bool { + if hasSkillsOnDisk(globalDir) { + return true + } + homeDir, err := env.UserHomeDir(ctx) + if err != nil { + return false + } + return hasSkillsOnDisk(filepath.Join(homeDir, ".databricks", "agent-skills")) +} + +// sortedKeys returns the keys of a map sorted alphabetically. +func sortedKeys[V any](m map[string]V) []string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +// FormatUpdateResult returns a human-readable summary of the update result. +// When check is true, output uses "Would update/add" instead of "Updated/Added". +func FormatUpdateResult(result *UpdateResult, check bool) string { + var lines []string + + updateVerb := "updated" + addVerb := "added" + summaryVerb := "Updated" + if check { + updateVerb = "would update" + addVerb = "would add" + summaryVerb = "Would update" + } + + for _, u := range result.Updated { + if u.OldVersion == "" { + lines = append(lines, fmt.Sprintf(" %s %s -> v%s", updateVerb, u.Name, u.NewVersion)) + } else { + lines = append(lines, fmt.Sprintf(" %s %s v%s -> v%s", updateVerb, u.Name, u.OldVersion, u.NewVersion)) + } + } + + for _, a := range result.Added { + lines = append(lines, fmt.Sprintf(" %s %s v%s", addVerb, a.Name, a.NewVersion)) + } + + total := len(result.Updated) + len(result.Added) + if total == 0 { + return "No changes." + } + + noun := "skills" + if total == 1 { + noun = "skill" + } + lines = append(lines, fmt.Sprintf("%s %d %s.", summaryVerb, total, noun)) + return strings.Join(lines, "\n") +} diff --git a/experimental/aitools/lib/installer/update_test.go b/experimental/aitools/lib/installer/update_test.go new file mode 100644 index 0000000000..f5e98ee5eb --- /dev/null +++ b/experimental/aitools/lib/installer/update_test.go @@ -0,0 +1,415 @@ +package installer + +import ( + "bytes" + "context" + "errors" + "log/slog" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/log" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestUpdateNoStateReturnsInstallHint(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + _ = tmp + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + _, err := UpdateSkills(ctx, src, nil, UpdateOptions{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "no skills installed") + assert.Contains(t, err.Error(), "databricks experimental aitools install") +} + +func TestUpdateLegacyInstallDetected(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + + // Create skills in canonical location but no state file. + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + require.NoError(t, os.MkdirAll(filepath.Join(globalDir, "databricks-sql"), 0o755)) + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + _, err := UpdateSkills(ctx, src, nil, UpdateOptions{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "previous install without state tracking") + assert.Contains(t, err.Error(), "refresh before updating") +} + +func TestUpdateAlreadyUpToDate(t *testing.T) { + tmp := setupTestHome(t) + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + setupFetchMock(t) + + // Install first. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // Reset stderr. + stderr.Reset() + + // Update with same release. + result, err := UpdateSkills(ctx, src, []*agents.Agent{agent}, UpdateOptions{}) + require.NoError(t, err) + assert.Contains(t, stderr.String(), "Already up to date.") + assert.Len(t, result.Unchanged, 2) + assert.Empty(t, result.Updated) + assert.Empty(t, result.Added) +} + +func TestUpdateVersionDiffDetected(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + // Install v0.1.0. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // Updated manifest with new version for one skill. + updatedManifest := testManifest() + updatedManifest.Skills["databricks-sql"] = SkillMeta{ + Version: "0.2.0", + Files: []string{"SKILL.md"}, + } + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + + result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) + require.NoError(t, err) + + require.Len(t, result.Updated, 1) + assert.Equal(t, "databricks-sql", result.Updated[0].Name) + assert.Equal(t, "0.1.0", result.Updated[0].OldVersion) + assert.Equal(t, "0.2.0", result.Updated[0].NewVersion) + + // databricks-jobs unchanged. + assert.Contains(t, result.Unchanged, "databricks-jobs") + + // State should be updated. + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.Equal(t, "v0.2.0", state.Release) + assert.Equal(t, "0.2.0", state.Skills["databricks-sql"]) +} + +func TestUpdateCheckDryRun(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + // Install v0.1.0. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // Updated manifest. + updatedManifest := testManifest() + updatedManifest.Skills["databricks-sql"] = SkillMeta{ + Version: "0.2.0", + Files: []string{"SKILL.md"}, + } + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + + // Track fetch calls to verify no downloads happen. + fetchCalls := 0 + orig := fetchFileFn + t.Cleanup(func() { fetchFileFn = orig }) + fetchFileFn = func(_ context.Context, _, _, _ string) ([]byte, error) { + fetchCalls++ + return []byte("content"), nil + } + + result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{Check: true}) + require.NoError(t, err) + + // Should report the diff. + require.Len(t, result.Updated, 1) + assert.Equal(t, "databricks-sql", result.Updated[0].Name) + + // Should NOT have downloaded anything. + assert.Equal(t, 0, fetchCalls) + + // State should be unchanged. + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.Equal(t, "v0.1.0", state.Release) +} + +func TestUpdateForceRedownloads(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + // Install v0.1.0. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // Track fetch calls on forced update (same release). + fetchCalls := 0 + orig := fetchFileFn + t.Cleanup(func() { fetchFileFn = orig }) + fetchFileFn = func(_ context.Context, _, _, _ string) ([]byte, error) { + fetchCalls++ + return []byte("content"), nil + } + + result, err := UpdateSkills(ctx, src, []*agents.Agent{agent}, UpdateOptions{Force: true}) + require.NoError(t, err) + + // All skills should be in Updated since Force re-downloads everything. + assert.Len(t, result.Updated, 2) + assert.Positive(t, fetchCalls, "force should trigger downloads") +} + +func TestUpdateAutoAddsNewSkills(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + // Install v0.1.0 with two skills. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // New manifest with an additional skill. + updatedManifest := testManifest() + updatedManifest.Skills["databricks-notebooks"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + } + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + + result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) + require.NoError(t, err) + + // The new skill should be in Added. + require.Len(t, result.Added, 1) + assert.Equal(t, "databricks-notebooks", result.Added[0].Name) + assert.Equal(t, "0.1.0", result.Added[0].NewVersion) + + // State should include the new skill. + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.Equal(t, "0.1.0", state.Skills["databricks-notebooks"]) +} + +func TestUpdateNoNewIgnoresNewSkills(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + // Install v0.1.0. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // New manifest with an additional skill. + updatedManifest := testManifest() + updatedManifest.Skills["databricks-notebooks"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + } + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + + result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{NoNew: true}) + require.NoError(t, err) + + // No new skills should be added. + assert.Empty(t, result.Added) + // Existing skills should be unchanged (same version). + assert.Len(t, result.Unchanged, 2) + + // State should NOT include the new skill. + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.NotContains(t, state.Skills, "databricks-notebooks") +} + +func TestUpdateOutputSortedAlphabetically(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + // Install with skills. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // Update all skills. + updatedManifest := testManifest() + updatedManifest.Skills["databricks-sql"] = SkillMeta{Version: "0.2.0", Files: []string{"SKILL.md"}} + updatedManifest.Skills["databricks-jobs"] = SkillMeta{Version: "0.2.0", Files: []string{"SKILL.md"}} + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + + result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) + require.NoError(t, err) + + require.Len(t, result.Updated, 2) + assert.Equal(t, "databricks-jobs", result.Updated[0].Name) + assert.Equal(t, "databricks-sql", result.Updated[1].Name) +} + +// nonAuthoritativeMock returns a fallback ref with authoritative=false. +type nonAuthoritativeMock struct{} + +func (m *nonAuthoritativeMock) FetchManifest(_ context.Context, _ string) (*Manifest, error) { + return nil, errors.New("should not be called") +} + +func (m *nonAuthoritativeMock) FetchLatestRelease(_ context.Context) (string, bool, error) { + return defaultSkillsRepoRef, false, nil +} + +func TestUpdateNonAuthoritativeWithoutForce(t *testing.T) { + tmp := setupTestHome(t) + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + setupFetchMock(t) + + // Install first. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + stderr.Reset() + + // Non-authoritative release fetch (offline fallback). + fallbackSrc := &nonAuthoritativeMock{} + result, err := UpdateSkills(ctx, fallbackSrc, []*agents.Agent{agent}, UpdateOptions{}) + require.NoError(t, err) + assert.Contains(t, stderr.String(), "Could not check for updates (offline?)") + assert.Len(t, result.Unchanged, 2) +} + +func TestUpdateSkillRemovedFromManifestWarning(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + // Capture log output to verify warning. + var logBuf bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logBuf, &slog.HandlerOptions{Level: slog.LevelWarn})) + ctx = log.NewContext(ctx, logger) + + // Install v0.1.0 with two skills. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // New manifest that removed databricks-jobs. + updatedManifest := &Manifest{ + Version: "1", + UpdatedAt: "2024-02-01", + Skills: map[string]SkillMeta{ + "databricks-sql": {Version: "0.2.0", Files: []string{"SKILL.md"}}, + }, + } + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + + result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) + require.NoError(t, err) + + // databricks-jobs should be kept as unchanged. + assert.Contains(t, result.Unchanged, "databricks-jobs") + // Warning should be logged. + assert.Contains(t, logBuf.String(), "databricks-jobs") + assert.Contains(t, logBuf.String(), "not found in manifest v0.2.0") + + // State should still have databricks-jobs. + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.Contains(t, state.Skills, "databricks-jobs") +} + +func TestUpdateSkipsExperimentalSkills(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + // Install v0.1.0 (not experimental). + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // New manifest with an experimental skill. + updatedManifest := testManifest() + updatedManifest.Skills["databricks-experimental"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + Experimental: true, + } + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + + result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) + require.NoError(t, err) + + // Experimental skill should be skipped. + assert.Contains(t, result.Skipped, "databricks-experimental") + assert.Empty(t, result.Added) +} + +func TestUpdateSkipsMinCLIVersionSkills(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + setBuildVersion(t, "0.200.0") + + var logBuf bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logBuf, &slog.HandlerOptions{Level: slog.LevelWarn})) + ctx = log.NewContext(ctx, logger) + + // Install v0.1.0. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // New manifest with a skill requiring a newer CLI. + updatedManifest := testManifest() + updatedManifest.Skills["databricks-future"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + MinCLIVer: "0.300.0", + } + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + + result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) + require.NoError(t, err) + + assert.Contains(t, result.Skipped, "databricks-future") + assert.Contains(t, logBuf.String(), "requires CLI version 0.300.0") +} + +func TestFormatUpdateResultCheckMode(t *testing.T) { + result := &UpdateResult{ + Updated: []SkillUpdate{ + {Name: "databricks-sql", OldVersion: "0.1.0", NewVersion: "0.2.0"}, + }, + Added: []SkillUpdate{ + {Name: "databricks-notebooks", NewVersion: "0.1.0"}, + }, + } + + output := FormatUpdateResult(result, false) + assert.Contains(t, output, "updated databricks-sql") + assert.Contains(t, output, "added databricks-notebooks") + assert.Contains(t, output, "Updated 2 skills.") + + checkOutput := FormatUpdateResult(result, true) + assert.Contains(t, checkOutput, "would update databricks-sql") + assert.Contains(t, checkOutput, "would add databricks-notebooks") + assert.Contains(t, checkOutput, "Would update 2 skills.") +}