diff --git a/experimental/aitools/cmd/aitools.go b/experimental/aitools/cmd/aitools.go index 3058013dbe..3ce43a1073 100644 --- a/experimental/aitools/cmd/aitools.go +++ b/experimental/aitools/cmd/aitools.go @@ -20,6 +20,9 @@ Provides commands to: cmd.AddCommand(newInstallCmd()) cmd.AddCommand(newSkillsCmd()) cmd.AddCommand(newToolsCmd()) + cmd.AddCommand(newUpdateCmd()) + cmd.AddCommand(newUninstallCmd()) + cmd.AddCommand(newVersionCmd()) return cmd } diff --git a/experimental/aitools/cmd/uninstall.go b/experimental/aitools/cmd/uninstall.go new file mode 100644 index 0000000000..9edd360100 --- /dev/null +++ b/experimental/aitools/cmd/uninstall.go @@ -0,0 +1,19 @@ +package aitools + +import ( + "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/spf13/cobra" +) + +func newUninstallCmd() *cobra.Command { + return &cobra.Command{ + Use: "uninstall", + Short: "Uninstall all AI skills", + Long: `Remove all installed Databricks AI skills from all coding agents. + +Removes skill directories, symlinks, and the state file.`, + RunE: func(cmd *cobra.Command, args []string) error { + return installer.UninstallSkills(cmd.Context()) + }, + } +} diff --git a/experimental/aitools/cmd/update.go b/experimental/aitools/cmd/update.go new file mode 100644 index 0000000000..67dd2916cf --- /dev/null +++ b/experimental/aitools/cmd/update.go @@ -0,0 +1,44 @@ +package aitools + +import ( + "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 + + 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{} + result, err := installer.UpdateSkills(ctx, src, installed, installer.UpdateOptions{ + Check: check, + Force: force, + NoNew: noNew, + }) + 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") + return cmd +} diff --git a/experimental/aitools/cmd/version.go b/experimental/aitools/cmd/version.go new file mode 100644 index 0000000000..4dc91beb3b --- /dev/null +++ b/experimental/aitools/cmd/version.go @@ -0,0 +1,84 @@ +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 { + return &cobra.Command{ + Use: "version", + Short: "Show installed AI skills version", + RunE: func(cmd *cobra.Command, args []string) error { + 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 + }, + } +} diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go index 3a1241c2ce..bfce08d6fa 100644 --- a/experimental/aitools/lib/installer/installer.go +++ b/experimental/aitools/lib/installer/installer.go @@ -116,7 +116,7 @@ func ListSkills(ctx context.Context) error { // 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) + latestTag, _, err := src.FetchLatestRelease(ctx) if err != nil { return fmt.Errorf("failed to fetch latest release: %w", err) } diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go index 4a5970002b..a7ee7bc742 100644 --- a/experimental/aitools/lib/installer/installer_test.go +++ b/experimental/aitools/lib/installer/installer_test.go @@ -18,9 +18,10 @@ import ( // mockManifestSource is a test double for ManifestSource. type mockManifestSource struct { - manifest *Manifest - release string - fetchErr error + manifest *Manifest + release string + authoritative bool + fetchErr error } func (m *mockManifestSource) FetchManifest(_ context.Context, _ string) (*Manifest, error) { @@ -30,8 +31,8 @@ func (m *mockManifestSource) FetchManifest(_ context.Context, _ string) (*Manife return m.manifest, nil } -func (m *mockManifestSource) FetchLatestRelease(_ context.Context) (string, error) { - return m.release, nil +func (m *mockManifestSource) FetchLatestRelease(_ context.Context) (string, bool, error) { + return m.release, m.authoritative, nil } func testManifest() *Manifest { @@ -195,7 +196,7 @@ func TestInstallSkillsForAgentsWritesState(t *testing.T) { ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"} + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} agent := testAgent(tmp) err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) @@ -219,7 +220,7 @@ func TestInstallSkillForSingleWritesState(t *testing.T) { ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"} + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} agent := testAgent(tmp) err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{ @@ -242,7 +243,7 @@ func TestInstallSkillsSpecificNotFound(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"} + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} agent := testAgent(tmp) err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{ @@ -264,7 +265,7 @@ func TestExperimentalSkillsSkippedByDefault(t *testing.T) { Experimental: true, } - src := &mockManifestSource{manifest: manifest, release: "v0.1.0"} + src := &mockManifestSource{manifest: manifest, release: "v0.1.0", authoritative: true} agent := testAgent(tmp) err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) @@ -292,7 +293,7 @@ func TestExperimentalSkillsIncludedWithFlag(t *testing.T) { Experimental: true, } - src := &mockManifestSource{manifest: manifest, release: "v0.1.0"} + src := &mockManifestSource{manifest: manifest, release: "v0.1.0", authoritative: true} agent := testAgent(tmp) err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{ @@ -328,7 +329,7 @@ func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) { MinCLIVer: "0.300.0", } - src := &mockManifestSource{manifest: manifest, release: "v0.1.0"} + src := &mockManifestSource{manifest: manifest, release: "v0.1.0", authoritative: true} agent := testAgent(tmp) err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) @@ -358,7 +359,7 @@ func TestMinCLIVersionHardErrorForInstallSingle(t *testing.T) { MinCLIVer: "0.300.0", } - src := &mockManifestSource{manifest: manifest, release: "v0.1.0"} + src := &mockManifestSource{manifest: manifest, release: "v0.1.0", authoritative: true} agent := testAgent(tmp) err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{ @@ -374,7 +375,7 @@ func TestIdempotentSecondInstallSkips(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"} + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} agent := testAgent(tmp) // First install. @@ -403,7 +404,7 @@ func TestIdempotentInstallUpdatesNewVersions(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"} + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} agent := testAgent(tmp) // First install. @@ -416,7 +417,7 @@ func TestIdempotentInstallUpdatesNewVersions(t *testing.T) { Version: "0.2.0", Files: []string{"SKILL.md"}, } - src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0"} + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} // Track which skills are fetched. var fetchedSkills []string @@ -452,7 +453,7 @@ func TestLegacyDetectMessagePrinted(t *testing.T) { 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"} + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} agent := testAgent(tmp) err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) @@ -470,7 +471,7 @@ func TestLegacyDetectLegacyDir(t *testing.T) { 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"} + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} agent := testAgent(tmp) err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{}) diff --git a/experimental/aitools/lib/installer/source.go b/experimental/aitools/lib/installer/source.go index 59039b8b80..c375d108d4 100644 --- a/experimental/aitools/lib/installer/source.go +++ b/experimental/aitools/lib/installer/source.go @@ -16,12 +16,12 @@ 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. - // Implementations should fall back to a default ref on network errors rather - // than returning an error. The error return exists for cases where fallback is - // not possible (e.g., mock implementations in tests that want to simulate hard - // failures). - FetchLatestRelease(ctx context.Context) (string, 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. @@ -58,15 +58,16 @@ func (s *GitHubManifestSource) FetchManifest(ctx context.Context, ref string) (* } // FetchLatestRelease returns the latest release tag from GitHub. -// If DATABRICKS_SKILLS_REF is set, it is returned immediately. -// On any error (network, non-200, parse), falls back to defaultSkillsRepoRef. +// 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, error) { +func (s *GitHubManifestSource) FetchLatestRelease(ctx context.Context) (string, bool, error) { if ref := env.Get(ctx, "DATABRICKS_SKILLS_REF"); ref != "" { - return ref, nil + return ref, true, nil } url := fmt.Sprintf("https://api.github.com/repos/%s/%s/releases/latest", @@ -75,20 +76,20 @@ func (s *GitHubManifestSource) FetchLatestRelease(ctx context.Context) (string, 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, nil + 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, nil + 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, nil + return defaultSkillsRepoRef, false, nil } var release struct { @@ -96,13 +97,13 @@ func (s *GitHubManifestSource) FetchLatestRelease(ctx context.Context) (string, } 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, nil + return defaultSkillsRepoRef, false, nil } if release.TagName == "" { log.Debugf(ctx, "Empty tag_name in release response, falling back to %s", defaultSkillsRepoRef) - return defaultSkillsRepoRef, nil + return defaultSkillsRepoRef, false, nil } - return release.TagName, nil + return release.TagName, true, nil } diff --git a/experimental/aitools/lib/installer/uninstall.go b/experimental/aitools/lib/installer/uninstall.go new file mode 100644 index 0000000000..1253cb8cfb --- /dev/null +++ b/experimental/aitools/lib/installer/uninstall.go @@ -0,0 +1,160 @@ +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" +) + +// UninstallSkills removes all installed skills, their symlinks, and the state file. +func UninstallSkills(ctx context.Context) 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") + } + + skillCount := len(state.Skills) + + // Remove skill directories and symlinks for each skill in state. + for name := range state.Skills { + canonicalDir := filepath.Join(globalDir, name) + + // Remove symlinks from agent directories (only symlinks pointing to canonical dir). + removeSymlinksFromAgents(ctx, name, canonicalDir) + + // Remove canonical skill directory. + if err := os.RemoveAll(canonicalDir); err != nil { + log.Warnf(ctx, "Failed to remove %s: %v", canonicalDir, err) + } + } + + // Clean up orphaned symlinks pointing into the canonical dir. + cleanOrphanedSymlinks(ctx, globalDir) + + // Delete state file. + 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) + } + + noun := "skills" + if skillCount == 1 { + noun = "skill" + } + cmdio.LogString(ctx, fmt.Sprintf("Uninstalled %d %s.", skillCount, 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..8b9e09c360 --- /dev/null +++ b/experimental/aitools/lib/installer/uninstall_test.go @@ -0,0 +1,257 @@ +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.") +} diff --git a/experimental/aitools/lib/installer/update.go b/experimental/aitools/lib/installer/update.go new file mode 100644 index 0000000000..48f7290223 --- /dev/null +++ b/experimental/aitools/lib/installer/update.go @@ -0,0 +1,259 @@ +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 := make([]SkillUpdate, 0, len(result.Updated)+len(result.Added)) + allChanges = append(allChanges, result.Updated...) + allChanges = append(allChanges, 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.") +}