From feafea4bc7a7a9e2602d1bffcac63ee42548dc27 Mon Sep 17 00:00:00 2001 From: simon Date: Sun, 22 Mar 2026 09:16:56 +0100 Subject: [PATCH 1/7] Add update, uninstall, and version commands for aitools skills Adds three new commands to the aitools skill management: - `update`: Updates installed skills to latest release with --check (dry run), --force, and --no-new flags. Auto-adds new manifest skills by default. - `uninstall`: Removes all installed skills, symlinks from all registry agents, cleans orphaned symlinks, and deletes state file. - `version`: Shows installed version, skill count, staleness check against latest release. Co-authored-by: Isaac --- experimental/aitools/cmd/aitools.go | 3 + experimental/aitools/cmd/uninstall.go | 19 ++ experimental/aitools/cmd/update.go | 44 +++ experimental/aitools/cmd/version.go | 68 ++++ .../aitools/lib/installer/uninstall.go | 147 +++++++++ .../aitools/lib/installer/uninstall_test.go | 194 ++++++++++++ experimental/aitools/lib/installer/update.go | 223 +++++++++++++ .../aitools/lib/installer/update_test.go | 292 ++++++++++++++++++ 8 files changed, 990 insertions(+) create mode 100644 experimental/aitools/cmd/uninstall.go create mode 100644 experimental/aitools/cmd/update.go create mode 100644 experimental/aitools/cmd/version.go create mode 100644 experimental/aitools/lib/installer/uninstall.go create mode 100644 experimental/aitools/lib/installer/uninstall_test.go create mode 100644 experimental/aitools/lib/installer/update.go create mode 100644 experimental/aitools/lib/installer/update_test.go 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..510470a34d --- /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)) + } + 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..d184dedf58 --- /dev/null +++ b/experimental/aitools/cmd/version.go @@ -0,0 +1,68 @@ +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, "Databricks AI skills: not installed") + cmdio.LogString(ctx, "") + cmdio.LogString(ctx, "Run 'databricks experimental aitools install' to install.") + return nil + } + + version := strings.TrimPrefix(state.Release, "v") + cmdio.LogString(ctx, fmt.Sprintf("Databricks AI skills v%s", version)) + cmdio.LogString(ctx, fmt.Sprintf(" Skills installed: %d", len(state.Skills))) + cmdio.LogString(ctx, fmt.Sprintf(" Last updated: %s", state.LastUpdated.Format("2006-01-02"))) + + // Best-effort staleness check. + if env.Get(ctx, "DATABRICKS_SKILLS_REF") != "" { + cmdio.LogString(ctx, " Using custom ref: $DATABRICKS_SKILLS_REF") + return nil + } + + src := &installer.GitHubManifestSource{} + latest, err := src.FetchLatestRelease(ctx) + if err != nil { + log.Debugf(ctx, "Could not check for updates: %v", err) + return nil + } + + if latest == state.Release { + cmdio.LogString(ctx, " Status: up to date") + } else { + latestVersion := strings.TrimPrefix(latest, "v") + cmdio.LogString(ctx, fmt.Sprintf(" Status: update available (v%s)", latestVersion)) + cmdio.LogString(ctx, "") + cmdio.LogString(ctx, "Run 'databricks experimental aitools update' to update.") + } + + return nil + }, + } +} diff --git a/experimental/aitools/lib/installer/uninstall.go b/experimental/aitools/lib/installer/uninstall.go new file mode 100644 index 0000000000..e03e749730 --- /dev/null +++ b/experimental/aitools/lib/installer/uninstall.go @@ -0,0 +1,147 @@ +package installer + +import ( + "context" + "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 fmt.Errorf("found skills from a previous install without state tracking; run 'databricks experimental aitools install' first, then uninstall") + } + return fmt.Errorf("no skills installed") + } + + skillCount := len(state.Skills) + + // Remove skill directories and symlinks for each skill in state. + for name := range state.Skills { + // Remove canonical skill directory. + canonicalDir := filepath.Join(globalDir, name) + if err := os.RemoveAll(canonicalDir); err != nil { + log.Warnf(ctx, "Failed to remove %s: %v", canonicalDir, err) + } + + // Remove symlinks from ALL agent directories (not just detected ones). + removeSymlinksFromAgents(ctx, name) + } + + // 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. +func removeSymlinksFromAgents(ctx context.Context, skillName 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 + } + + // Remove symlinks and directories alike. + if fi.Mode()&os.ModeSymlink != 0 { + if err := os.Remove(destDir); err != nil { + log.Warnf(ctx, "Failed to remove symlink %s: %v", destDir, err) + } + } else { + if err := os.RemoveAll(destDir); err != nil { + log.Warnf(ctx, "Failed to remove %s: %v", destDir, err) + } + } + + 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..6f0e203209 --- /dev/null +++ b/experimental/aitools/lib/installer/uninstall_test.go @@ -0,0 +1,194 @@ +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"} + 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"} + 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 TestUninstallHandlesBrokenSymlinks(t *testing.T) { + tmp := setupTestHome(t) + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + + // Create state and a broken symlink. + 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 broken symlink in a registry agent dir (.claude is in agents.Registry). + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".claude"), 0o755)) + agentSkillsDir := filepath.Join(tmp, ".claude", "skills") + require.NoError(t, os.MkdirAll(agentSkillsDir, 0o755)) + brokenLink := filepath.Join(agentSkillsDir, "databricks-sql") + require.NoError(t, os.Symlink("/nonexistent/target", brokenLink)) + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + err := UninstallSkills(ctx) + require.NoError(t, err) + + // Broken symlink should be removed. + _, err = os.Lstat(brokenLink) + assert.True(t, os.IsNotExist(err)) + 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..0488acc3c0 --- /dev/null +++ b/experimental/aitools/lib/installer/update.go @@ -0,0 +1,223 @@ +package installer + +import ( + "context" + "fmt" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" +) + +// 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, fmt.Errorf("found skills from a previous install without state tracking; run 'databricks experimental aitools install' to refresh before updating") + } + return nil, fmt.Errorf("no skills installed; run 'databricks experimental aitools install' to install") + } + + latestTag, err := src.FetchLatestRelease(ctx) + if err != nil { + if opts.Check { + log.Warnf(ctx, "Could not check for updates: %v", err) + return &UpdateResult{}, nil + } + return nil, fmt.Errorf("failed to fetch latest release: %w", err) + } + + 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{} + + // Sort skill names for deterministic output. + names := sortedKeys(skillSet) + + for _, name := range names { + meta, inManifest := manifest.Skills[name] + oldVersion := state.Skills[name] + + if !inManifest { + // Skill was in state but removed from manifest. Keep as unchanged. + result.Unchanged = append(result.Unchanged, 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. +func FormatUpdateResult(result *UpdateResult) string { + var lines []string + + for _, u := range result.Updated { + if u.OldVersion == "" { + lines = append(lines, fmt.Sprintf(" updated %s -> v%s", u.Name, u.NewVersion)) + } else { + lines = append(lines, fmt.Sprintf(" updated %s v%s -> v%s", u.Name, u.OldVersion, u.NewVersion)) + } + } + + for _, a := range result.Added { + lines = append(lines, fmt.Sprintf(" added %s v%s", 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("Updated %d %s.", 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..c452ec4e42 --- /dev/null +++ b/experimental/aitools/lib/installer/update_test.go @@ -0,0 +1,292 @@ +package installer + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + + "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 TestUpdateNoStateReturnsInstallHint(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + _ = tmp + + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"} + _, 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"} + _, 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"} + 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"} + 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"} + + 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"} + 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"} + + // 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"} + 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.True(t, fetchCalls > 0, "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"} + 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"} + + 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"} + 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"} + + 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"} + 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"} + + 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) +} + +// failingReleaseMock always fails on FetchLatestRelease. +type failingReleaseMock struct { + releaseErr error +} + +func (m *failingReleaseMock) FetchManifest(_ context.Context, _ string) (*Manifest, error) { + return nil, fmt.Errorf("should not be called") +} + +func (m *failingReleaseMock) FetchLatestRelease(_ context.Context) (string, error) { + return "", m.releaseErr +} + +func TestUpdateCheckWithNetworkFailure(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + + // Install first. + src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"} + agent := testAgent(tmp) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + + // Simulate network failure on release fetch. + failSrc := &failingReleaseMock{releaseErr: fmt.Errorf("network error")} + + result, err := UpdateSkills(ctx, failSrc, []*agents.Agent{agent}, UpdateOptions{Check: true}) + require.NoError(t, err, "check mode should not error on network failure") + assert.NotNil(t, result) +} From 6b39c76a0af514d187b1d0012348ee77ba6d8c16 Mon Sep 17 00:00:00 2001 From: simon Date: Sun, 22 Mar 2026 09:31:27 +0100 Subject: [PATCH 2/7] Fix PR3 review findings: authoritative release, filtering, symlink safety, output format - FetchLatestRelease returns (string, bool, error) so callers can distinguish real API responses from fallback defaults. Update and version commands use the authoritative flag to gate staleness checks. - Update now filters experimental and min_cli_version skills using the same logic as install. Warns when a skill is removed from the manifest. - Uninstall only removes symlinks pointing into the canonical skills dir, preserving user-managed directories and external symlinks. - FormatUpdateResult accepts a check flag to use "Would update" wording. - Version output matches the spec format (Databricks AI Tools header, skill count, staleness status on the same line). - Consistent "no install" messaging across update, uninstall, and version. - Added tests for removed-skill warning, experimental/min_cli_version filtering in update, check-mode output, and symlink-only uninstall. Co-authored-by: Isaac --- experimental/aitools/cmd/update.go | 2 +- experimental/aitools/cmd/version.go | 34 +++- .../aitools/lib/installer/uninstall.go | 45 +++-- .../aitools/lib/installer/uninstall_test.go | 81 +++++++- experimental/aitools/lib/installer/update.go | 57 ++++-- .../aitools/lib/installer/update_test.go | 181 +++++++++++++++--- 6 files changed, 324 insertions(+), 76 deletions(-) diff --git a/experimental/aitools/cmd/update.go b/experimental/aitools/cmd/update.go index 510470a34d..67dd2916cf 100644 --- a/experimental/aitools/cmd/update.go +++ b/experimental/aitools/cmd/update.go @@ -31,7 +31,7 @@ preview what would change without downloading.`, return err } if result != nil && (len(result.Updated) > 0 || len(result.Added) > 0) { - cmdio.LogString(ctx, installer.FormatUpdateResult(result)) + cmdio.LogString(ctx, installer.FormatUpdateResult(result, check)) } return nil }, diff --git a/experimental/aitools/cmd/version.go b/experimental/aitools/cmd/version.go index d184dedf58..1b752d5f1a 100644 --- a/experimental/aitools/cmd/version.go +++ b/experimental/aitools/cmd/version.go @@ -29,35 +29,51 @@ func newVersionCmd() *cobra.Command { } if state == nil { - cmdio.LogString(ctx, "Databricks AI skills: not installed") + cmdio.LogString(ctx, "No Databricks AI Tools components installed.") cmdio.LogString(ctx, "") - cmdio.LogString(ctx, "Run 'databricks experimental aitools install' to install.") + cmdio.LogString(ctx, "Run 'databricks experimental aitools install' to get started.") return nil } version := strings.TrimPrefix(state.Release, "v") - cmdio.LogString(ctx, fmt.Sprintf("Databricks AI skills v%s", version)) - cmdio.LogString(ctx, fmt.Sprintf(" Skills installed: %d", len(state.Skills))) - cmdio.LogString(ctx, fmt.Sprintf(" Last updated: %s", state.LastUpdated.Format("2006-01-02"))) + skillNoun := "skills" + if len(state.Skills) == 1 { + skillNoun = "skill" + } // Best-effort staleness check. if env.Get(ctx, "DATABRICKS_SKILLS_REF") != "" { - cmdio.LogString(ctx, " Using custom ref: $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, fmt.Sprintf(" Last updated: %s", state.LastUpdated.Format("2006-01-02"))) + cmdio.LogString(ctx, " Using custom ref: $DATABRICKS_SKILLS_REF") return nil } src := &installer.GitHubManifestSource{} - latest, err := src.FetchLatestRelease(ctx) + 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, fmt.Sprintf(" Last updated: %s", state.LastUpdated.Format("2006-01-02"))) + cmdio.LogString(ctx, " Could not check for latest version.") return nil } if latest == state.Release { - cmdio.LogString(ctx, " Status: up to date") + cmdio.LogString(ctx, fmt.Sprintf(" Skills: v%s (%d %s, up to date)", version, len(state.Skills), skillNoun)) + cmdio.LogString(ctx, fmt.Sprintf(" Last updated: %s", state.LastUpdated.Format("2006-01-02"))) } else { latestVersion := strings.TrimPrefix(latest, "v") - cmdio.LogString(ctx, fmt.Sprintf(" Status: update available (v%s)", latestVersion)) + cmdio.LogString(ctx, fmt.Sprintf(" Skills: v%s (%d %s)", version, len(state.Skills), skillNoun)) + cmdio.LogString(ctx, fmt.Sprintf(" Update available: v%s", latestVersion)) + cmdio.LogString(ctx, fmt.Sprintf(" Last updated: %s", state.LastUpdated.Format("2006-01-02"))) cmdio.LogString(ctx, "") cmdio.LogString(ctx, "Run 'databricks experimental aitools update' to update.") } diff --git a/experimental/aitools/lib/installer/uninstall.go b/experimental/aitools/lib/installer/uninstall.go index e03e749730..f472204d52 100644 --- a/experimental/aitools/lib/installer/uninstall.go +++ b/experimental/aitools/lib/installer/uninstall.go @@ -35,14 +35,15 @@ func UninstallSkills(ctx context.Context) error { // Remove skill directories and symlinks for each skill in state. for name := range state.Skills { - // Remove canonical skill directory. 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) } - - // Remove symlinks from ALL agent directories (not just detected ones). - removeSymlinksFromAgents(ctx, name) } // Clean up orphaned symlinks pointing into the canonical dir. @@ -62,8 +63,10 @@ func UninstallSkills(ctx context.Context) error { return nil } -// removeSymlinksFromAgents removes a skill's symlink from all agent directories in the registry. -func removeSymlinksFromAgents(ctx context.Context, skillName string) { +// 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) @@ -83,18 +86,28 @@ func removeSymlinksFromAgents(ctx context.Context, skillName string) { continue } - // Remove symlinks and directories alike. - if fi.Mode()&os.ModeSymlink != 0 { - if err := os.Remove(destDir); err != nil { - log.Warnf(ctx, "Failed to remove symlink %s: %v", destDir, err) - } - } else { - if err := os.RemoveAll(destDir); err != nil { - log.Warnf(ctx, "Failed to remove %s: %v", destDir, err) - } + if fi.Mode()&os.ModeSymlink == 0 { + log.Debugf(ctx, "Skipping non-symlink %s for %s", destDir, agent.DisplayName) + continue } - log.Debugf(ctx, "Removed %q from %s", skillName, agent.DisplayName) + 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) + } } } diff --git a/experimental/aitools/lib/installer/uninstall_test.go b/experimental/aitools/lib/installer/uninstall_test.go index 6f0e203209..8b9e09c360 100644 --- a/experimental/aitools/lib/installer/uninstall_test.go +++ b/experimental/aitools/lib/installer/uninstall_test.go @@ -18,7 +18,7 @@ func installTestSkills(t *testing.T, tmp string) string { 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) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) @@ -68,7 +68,7 @@ func TestUninstallRemovesSymlinks(t *testing.T) { }, } - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"} + 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()) @@ -161,11 +161,11 @@ func TestUninstallHandlesMissingDirectories(t *testing.T) { assert.Contains(t, stderr.String(), "Uninstalled 2 skills.") } -func TestUninstallHandlesBrokenSymlinks(t *testing.T) { +func TestUninstallHandlesBrokenSymlinksToCanonicalDir(t *testing.T) { tmp := setupTestHome(t) globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") - // Create state and a broken symlink. + // Create state with one skill. state := &InstallState{ SchemaVersion: 1, Release: "v0.1.0", @@ -176,19 +176,82 @@ func TestUninstallHandlesBrokenSymlinks(t *testing.T) { } require.NoError(t, SaveState(globalDir, state)) - // Create a broken symlink in a registry agent dir (.claude is in agents.Registry). + // 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)) - brokenLink := filepath.Join(agentSkillsDir, "databricks-sql") - require.NoError(t, os.Symlink("/nonexistent/target", brokenLink)) + 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) - // Broken symlink should be removed. - _, err = os.Lstat(brokenLink) + // 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 index 0488acc3c0..2f96dd010f 100644 --- a/experimental/aitools/lib/installer/update.go +++ b/experimental/aitools/lib/installer/update.go @@ -9,9 +9,11 @@ import ( "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. @@ -53,18 +55,19 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent if hasLegacyInstall(ctx, globalDir) { return nil, fmt.Errorf("found skills from a previous install without state tracking; run 'databricks experimental aitools install' to refresh before updating") } - return nil, fmt.Errorf("no skills installed; run 'databricks experimental aitools install' to install") + return nil, fmt.Errorf("no skills installed. Run 'databricks experimental aitools install' to install") } - latestTag, err := src.FetchLatestRelease(ctx) + latestTag, authoritative, err := src.FetchLatestRelease(ctx) if err != nil { - if opts.Check { - log.Warnf(ctx, "Could not check for updates: %v", err) - return &UpdateResult{}, 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 @@ -84,6 +87,9 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent result := &UpdateResult{} + cliVersion := build.GetInfo().Version + isDev := strings.HasPrefix(cliVersion, build.DefaultSemver) + // Sort skill names for deterministic output. names := sortedKeys(skillSet) @@ -92,11 +98,28 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent oldVersion := state.Skills[name] if !inManifest { - // Skill was in state but removed from manifest. Keep as unchanged. + _, 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] @@ -194,19 +217,29 @@ func sortedKeys[V any](m map[string]V) []string { } // FormatUpdateResult returns a human-readable summary of the update result. -func FormatUpdateResult(result *UpdateResult) string { +// 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(" updated %s -> v%s", u.Name, u.NewVersion)) + lines = append(lines, fmt.Sprintf(" %s %s -> v%s", updateVerb, u.Name, u.NewVersion)) } else { - lines = append(lines, fmt.Sprintf(" updated %s v%s -> v%s", u.Name, u.OldVersion, u.NewVersion)) + 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(" added %s v%s", a.Name, a.NewVersion)) + lines = append(lines, fmt.Sprintf(" %s %s v%s", addVerb, a.Name, a.NewVersion)) } total := len(result.Updated) + len(result.Added) @@ -218,6 +251,6 @@ func FormatUpdateResult(result *UpdateResult) string { if total == 1 { noun = "skill" } - lines = append(lines, fmt.Sprintf("Updated %d %s.", total, noun)) + 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 index c452ec4e42..420ff3487f 100644 --- a/experimental/aitools/lib/installer/update_test.go +++ b/experimental/aitools/lib/installer/update_test.go @@ -1,14 +1,17 @@ package installer import ( + "bytes" "context" "fmt" + "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" ) @@ -18,7 +21,7 @@ func TestUpdateNoStateReturnsInstallHint(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) _ = tmp - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"} + 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") @@ -33,7 +36,7 @@ func TestUpdateLegacyInstallDetected(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} _, err := UpdateSkills(ctx, src, nil, UpdateOptions{}) require.Error(t, err) assert.Contains(t, err.Error(), "previous install without state tracking") @@ -46,7 +49,7 @@ func TestUpdateAlreadyUpToDate(t *testing.T) { setupFetchMock(t) // Install first. - src := &mockManifestSource{manifest: testManifest(), release: "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{})) @@ -68,7 +71,7 @@ func TestUpdateVersionDiffDetected(t *testing.T) { setupFetchMock(t) // Install v0.1.0. - src := &mockManifestSource{manifest: testManifest(), release: "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{})) @@ -78,7 +81,7 @@ func TestUpdateVersionDiffDetected(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} result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) require.NoError(t, err) @@ -105,7 +108,7 @@ func TestUpdateCheckDryRun(t *testing.T) { setupFetchMock(t) // Install v0.1.0. - src := &mockManifestSource{manifest: testManifest(), release: "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{})) @@ -115,7 +118,7 @@ func TestUpdateCheckDryRun(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 fetch calls to verify no downloads happen. fetchCalls := 0 @@ -149,7 +152,7 @@ func TestUpdateForceRedownloads(t *testing.T) { setupFetchMock(t) // Install v0.1.0. - src := &mockManifestSource{manifest: testManifest(), release: "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{})) @@ -176,7 +179,7 @@ func TestUpdateAutoAddsNewSkills(t *testing.T) { setupFetchMock(t) // Install v0.1.0 with two skills. - src := &mockManifestSource{manifest: testManifest(), release: "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{})) @@ -186,7 +189,7 @@ func TestUpdateAutoAddsNewSkills(t *testing.T) { Version: "0.1.0", Files: []string{"SKILL.md"}, } - src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0"} + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) require.NoError(t, err) @@ -209,7 +212,7 @@ func TestUpdateNoNewIgnoresNewSkills(t *testing.T) { setupFetchMock(t) // Install v0.1.0. - src := &mockManifestSource{manifest: testManifest(), release: "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{})) @@ -219,7 +222,7 @@ func TestUpdateNoNewIgnoresNewSkills(t *testing.T) { Version: "0.1.0", Files: []string{"SKILL.md"}, } - src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0"} + 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) @@ -242,7 +245,7 @@ func TestUpdateOutputSortedAlphabetically(t *testing.T) { setupFetchMock(t) // Install with skills. - src := &mockManifestSource{manifest: testManifest(), release: "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{})) @@ -250,7 +253,7 @@ func TestUpdateOutputSortedAlphabetically(t *testing.T) { 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"} + src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) require.NoError(t, err) @@ -260,33 +263,153 @@ func TestUpdateOutputSortedAlphabetically(t *testing.T) { assert.Equal(t, "databricks-sql", result.Updated[1].Name) } -// failingReleaseMock always fails on FetchLatestRelease. -type failingReleaseMock struct { - releaseErr error -} +// nonAuthoritativeMock returns a fallback ref with authoritative=false. +type nonAuthoritativeMock struct{} -func (m *failingReleaseMock) FetchManifest(_ context.Context, _ string) (*Manifest, error) { +func (m *nonAuthoritativeMock) FetchManifest(_ context.Context, _ string) (*Manifest, error) { return nil, fmt.Errorf("should not be called") } -func (m *failingReleaseMock) FetchLatestRelease(_ context.Context) (string, error) { - return "", m.releaseErr +func (m *nonAuthoritativeMock) FetchLatestRelease(_ context.Context) (string, bool, error) { + return defaultSkillsRepoRef, false, nil } -func TestUpdateCheckWithNetworkFailure(t *testing.T) { +func TestUpdateNonAuthoritativeWithoutForce(t *testing.T) { tmp := setupTestHome(t) - ctx := cmdio.MockDiscard(t.Context()) + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) // Install first. - src := &mockManifestSource{manifest: testManifest(), release: "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{})) - // Simulate network failure on release fetch. - failSrc := &failingReleaseMock{releaseErr: fmt.Errorf("network error")} + 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.") - result, err := UpdateSkills(ctx, failSrc, []*agents.Agent{agent}, UpdateOptions{Check: true}) - require.NoError(t, err, "check mode should not error on network failure") - assert.NotNil(t, result) + 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.") } From 1c792c09c7b031eb80675e2dc57aac9f3ada8ecc Mon Sep 17 00:00:00 2001 From: simon Date: Sun, 22 Mar 2026 09:32:32 +0100 Subject: [PATCH 3/7] Fix trailing whitespace in uninstall.go --- experimental/aitools/lib/installer/uninstall.go | 1 - 1 file changed, 1 deletion(-) diff --git a/experimental/aitools/lib/installer/uninstall.go b/experimental/aitools/lib/installer/uninstall.go index f472204d52..cd0c723fdb 100644 --- a/experimental/aitools/lib/installer/uninstall.go +++ b/experimental/aitools/lib/installer/uninstall.go @@ -157,4 +157,3 @@ func cleanOrphanedSymlinks(ctx context.Context, globalDir string) { } } } - From 5f7baa43646f967d8f428b71fc6e505a74bbc048 Mon Sep 17 00:00:00 2001 From: simon Date: Sun, 22 Mar 2026 09:38:33 +0100 Subject: [PATCH 4/7] Fix lint issues: perfsprint, testifylint Co-authored-by: Isaac --- experimental/aitools/cmd/version.go | 10 +++++----- experimental/aitools/lib/installer/uninstall.go | 5 +++-- experimental/aitools/lib/installer/update.go | 5 +++-- experimental/aitools/lib/installer/update_test.go | 6 +++--- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/experimental/aitools/cmd/version.go b/experimental/aitools/cmd/version.go index 1b752d5f1a..4dc91beb3b 100644 --- a/experimental/aitools/cmd/version.go +++ b/experimental/aitools/cmd/version.go @@ -45,7 +45,7 @@ func newVersionCmd() *cobra.Command { 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, fmt.Sprintf(" Last updated: %s", state.LastUpdated.Format("2006-01-02"))) + cmdio.LogString(ctx, " Last updated: "+state.LastUpdated.Format("2006-01-02")) cmdio.LogString(ctx, " Using custom ref: $DATABRICKS_SKILLS_REF") return nil } @@ -61,19 +61,19 @@ func newVersionCmd() *cobra.Command { if !authoritative { cmdio.LogString(ctx, fmt.Sprintf(" Skills: v%s (%d %s)", version, len(state.Skills), skillNoun)) - cmdio.LogString(ctx, fmt.Sprintf(" Last updated: %s", state.LastUpdated.Format("2006-01-02"))) + 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, fmt.Sprintf(" Last updated: %s", state.LastUpdated.Format("2006-01-02"))) + 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, fmt.Sprintf(" Update available: v%s", latestVersion)) - cmdio.LogString(ctx, fmt.Sprintf(" Last updated: %s", state.LastUpdated.Format("2006-01-02"))) + 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.") } diff --git a/experimental/aitools/lib/installer/uninstall.go b/experimental/aitools/lib/installer/uninstall.go index cd0c723fdb..1253cb8cfb 100644 --- a/experimental/aitools/lib/installer/uninstall.go +++ b/experimental/aitools/lib/installer/uninstall.go @@ -2,6 +2,7 @@ package installer import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -26,9 +27,9 @@ func UninstallSkills(ctx context.Context) error { if state == nil { if hasLegacyInstall(ctx, globalDir) { - return fmt.Errorf("found skills from a previous install without state tracking; run 'databricks experimental aitools install' first, then uninstall") + return errors.New("found skills from a previous install without state tracking; run 'databricks experimental aitools install' first, then uninstall") } - return fmt.Errorf("no skills installed") + return errors.New("no skills installed") } skillCount := len(state.Skills) diff --git a/experimental/aitools/lib/installer/update.go b/experimental/aitools/lib/installer/update.go index 2f96dd010f..6ba461cd10 100644 --- a/experimental/aitools/lib/installer/update.go +++ b/experimental/aitools/lib/installer/update.go @@ -2,6 +2,7 @@ package installer import ( "context" + "errors" "fmt" "path/filepath" "sort" @@ -53,9 +54,9 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent if state == nil { if hasLegacyInstall(ctx, globalDir) { - return nil, fmt.Errorf("found skills from a previous install without state tracking; run 'databricks experimental aitools install' to refresh before updating") + return nil, errors.New("found skills from a previous install without state tracking; run 'databricks experimental aitools install' to refresh before updating") } - return nil, fmt.Errorf("no skills installed. Run 'databricks experimental aitools install' to install") + return nil, errors.New("no skills installed. Run 'databricks experimental aitools install' to install") } latestTag, authoritative, err := src.FetchLatestRelease(ctx) diff --git a/experimental/aitools/lib/installer/update_test.go b/experimental/aitools/lib/installer/update_test.go index 420ff3487f..f5e98ee5eb 100644 --- a/experimental/aitools/lib/installer/update_test.go +++ b/experimental/aitools/lib/installer/update_test.go @@ -3,7 +3,7 @@ package installer import ( "bytes" "context" - "fmt" + "errors" "log/slog" "os" "path/filepath" @@ -170,7 +170,7 @@ func TestUpdateForceRedownloads(t *testing.T) { // All skills should be in Updated since Force re-downloads everything. assert.Len(t, result.Updated, 2) - assert.True(t, fetchCalls > 0, "force should trigger downloads") + assert.Positive(t, fetchCalls, "force should trigger downloads") } func TestUpdateAutoAddsNewSkills(t *testing.T) { @@ -267,7 +267,7 @@ func TestUpdateOutputSortedAlphabetically(t *testing.T) { type nonAuthoritativeMock struct{} func (m *nonAuthoritativeMock) FetchManifest(_ context.Context, _ string) (*Manifest, error) { - return nil, fmt.Errorf("should not be called") + return nil, errors.New("should not be called") } func (m *nonAuthoritativeMock) FetchLatestRelease(_ context.Context) (string, bool, error) { From 8da78a44f96f7b7b920c529b8558f53a9b47f993 Mon Sep 17 00:00:00 2001 From: simon Date: Sun, 22 Mar 2026 11:39:01 +0100 Subject: [PATCH 5/7] Fix slice aliasing in update result formatting Co-authored-by: Isaac --- experimental/aitools/lib/installer/update.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/experimental/aitools/lib/installer/update.go b/experimental/aitools/lib/installer/update.go index 6ba461cd10..48f7290223 100644 --- a/experimental/aitools/lib/installer/update.go +++ b/experimental/aitools/lib/installer/update.go @@ -147,7 +147,9 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent } // Download and install updated/added skills. - allChanges := append(result.Updated, result.Added...) + 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 { From 0023f1b9fffa5abb058dae95adfdecf70fa2454d Mon Sep 17 00:00:00 2001 From: simon Date: Thu, 26 Mar 2026 00:07:22 +0100 Subject: [PATCH 6/7] Remove FetchLatestRelease, use GetSkillsRef for version resolution After squash-merge of PR2, FetchLatestRelease was replaced with getSkillsRef. Export it as GetSkillsRef and update all PR3 code (update.go, version.go, tests) to use the config-based ref instead of the removed GitHub API release lookup. Co-authored-by: Isaac --- experimental/aitools/cmd/version.go | 30 +---- .../aitools/lib/installer/installer.go | 8 +- .../aitools/lib/installer/uninstall_test.go | 4 +- experimental/aitools/lib/installer/update.go | 10 +- .../aitools/lib/installer/update_test.go | 113 ++++++++---------- 5 files changed, 64 insertions(+), 101 deletions(-) diff --git a/experimental/aitools/cmd/version.go b/experimental/aitools/cmd/version.go index 4dc91beb3b..0147194648 100644 --- a/experimental/aitools/cmd/version.go +++ b/experimental/aitools/cmd/version.go @@ -6,8 +6,6 @@ import ( "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" ) @@ -41,36 +39,14 @@ func newVersionCmd() *cobra.Command { 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 { + latestRef := installer.GetSkillsRef(ctx) + if latestRef == 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") + latestVersion := strings.TrimPrefix(latestRef, "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")) diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go index 14e9a3cf91..1f9226843d 100644 --- a/experimental/aitools/lib/installer/installer.go +++ b/experimental/aitools/lib/installer/installer.go @@ -32,7 +32,9 @@ const ( // It is a package-level var so tests can replace it with a mock. var fetchFileFn = fetchSkillFile -func getSkillsRef(ctx context.Context) string { +// GetSkillsRef returns the skills repo ref to use. If DATABRICKS_SKILLS_REF +// is set, it returns that value; otherwise it returns the default ref. +func GetSkillsRef(ctx context.Context) string { if ref := env.Get(ctx, "DATABRICKS_SKILLS_REF"); ref != "" { return ref } @@ -66,7 +68,7 @@ type InstallOptions struct { // This is a convenience wrapper that uses the default GitHubManifestSource. func FetchManifest(ctx context.Context) (*Manifest, error) { src := &GitHubManifestSource{} - ref := getSkillsRef(ctx) + ref := GetSkillsRef(ctx) return src.FetchManifest(ctx, ref) } @@ -117,7 +119,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 { - ref := getSkillsRef(ctx) + ref := GetSkillsRef(ctx) manifest, err := src.FetchManifest(ctx, ref) if err != nil { return err diff --git a/experimental/aitools/lib/installer/uninstall_test.go b/experimental/aitools/lib/installer/uninstall_test.go index 8b9e09c360..f7cbbffe35 100644 --- a/experimental/aitools/lib/installer/uninstall_test.go +++ b/experimental/aitools/lib/installer/uninstall_test.go @@ -18,7 +18,7 @@ func installTestSkills(t *testing.T, tmp string) string { ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) @@ -68,7 +68,7 @@ func TestUninstallRemovesSymlinks(t *testing.T) { }, } - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + src := &mockManifestSource{manifest: testManifest()} require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{claudeAgent, cursorAgent}, InstallOptions{})) ctx2, _ := cmdio.NewTestContextWithStderr(t.Context()) diff --git a/experimental/aitools/lib/installer/update.go b/experimental/aitools/lib/installer/update.go index 48f7290223..0c46de8f19 100644 --- a/experimental/aitools/lib/installer/update.go +++ b/experimental/aitools/lib/installer/update.go @@ -59,15 +59,7 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent 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 - } + latestTag := GetSkillsRef(ctx) if state.Release == latestTag && !opts.Force { cmdio.LogString(ctx, "Already up to date.") diff --git a/experimental/aitools/lib/installer/update_test.go b/experimental/aitools/lib/installer/update_test.go index f5e98ee5eb..862cf4a391 100644 --- a/experimental/aitools/lib/installer/update_test.go +++ b/experimental/aitools/lib/installer/update_test.go @@ -3,7 +3,6 @@ package installer import ( "bytes" "context" - "errors" "log/slog" "os" "path/filepath" @@ -21,7 +20,7 @@ func TestUpdateNoStateReturnsInstallHint(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) _ = tmp - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + src := &mockManifestSource{manifest: testManifest()} _, err := UpdateSkills(ctx, src, nil, UpdateOptions{}) require.Error(t, err) assert.Contains(t, err.Error(), "no skills installed") @@ -36,7 +35,7 @@ func TestUpdateLegacyInstallDetected(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", authoritative: true} + src := &mockManifestSource{manifest: testManifest()} _, err := UpdateSkills(ctx, src, nil, UpdateOptions{}) require.Error(t, err) assert.Contains(t, err.Error(), "previous install without state tracking") @@ -49,7 +48,7 @@ func TestUpdateAlreadyUpToDate(t *testing.T) { setupFetchMock(t) // Install first. - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) @@ -70,18 +69,21 @@ func TestUpdateVersionDiffDetected(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) - // Install v0.1.0. - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + // Install with default ref. + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + // Simulate a new release by changing the ref. + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.0") + // 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} + src2 := &mockManifestSource{manifest: updatedManifest} result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) require.NoError(t, err) @@ -107,18 +109,21 @@ func TestUpdateCheckDryRun(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) - // Install v0.1.0. - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + // Install with default ref. + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + // Simulate a new release. + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.0") + // 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} + src2 := &mockManifestSource{manifest: updatedManifest} // Track fetch calls to verify no downloads happen. fetchCalls := 0 @@ -139,11 +144,11 @@ func TestUpdateCheckDryRun(t *testing.T) { // Should NOT have downloaded anything. assert.Equal(t, 0, fetchCalls) - // State should be unchanged. + // State should be unchanged (still the original install ref). globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") state, err := LoadState(globalDir) require.NoError(t, err) - assert.Equal(t, "v0.1.0", state.Release) + assert.Equal(t, defaultSkillsRepoRef, state.Release) } func TestUpdateForceRedownloads(t *testing.T) { @@ -152,7 +157,7 @@ func TestUpdateForceRedownloads(t *testing.T) { setupFetchMock(t) // Install v0.1.0. - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) @@ -178,18 +183,21 @@ func TestUpdateAutoAddsNewSkills(t *testing.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} + // Install with default ref. + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + // Simulate a new release. + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.0") + // 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} + src2 := &mockManifestSource{manifest: updatedManifest} result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) require.NoError(t, err) @@ -211,18 +219,21 @@ func TestUpdateNoNewIgnoresNewSkills(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) - // Install v0.1.0. - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + // Install with default ref. + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + // Simulate a new release. + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.0") + // 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} + src2 := &mockManifestSource{manifest: updatedManifest} result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{NoNew: true}) require.NoError(t, err) @@ -245,15 +256,18 @@ func TestUpdateOutputSortedAlphabetically(t *testing.T) { setupFetchMock(t) // Install with skills. - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + // Simulate a new release. + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.0") + // 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} + src2 := &mockManifestSource{manifest: updatedManifest} result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) require.NoError(t, err) @@ -263,36 +277,6 @@ func TestUpdateOutputSortedAlphabetically(t *testing.T) { 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) @@ -304,11 +288,14 @@ func TestUpdateSkillRemovedFromManifestWarning(t *testing.T) { 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} + // Install with default ref. + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + // Simulate a new release. + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.0") + // New manifest that removed databricks-jobs. updatedManifest := &Manifest{ Version: "1", @@ -317,7 +304,7 @@ func TestUpdateSkillRemovedFromManifestWarning(t *testing.T) { "databricks-sql": {Version: "0.2.0", Files: []string{"SKILL.md"}}, }, } - src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + src2 := &mockManifestSource{manifest: updatedManifest} result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) require.NoError(t, err) @@ -340,11 +327,14 @@ func TestUpdateSkipsExperimentalSkills(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) - // Install v0.1.0 (not experimental). - src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0", authoritative: true} + // Install with default ref (not experimental). + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + // Simulate a new release. + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.0") + // New manifest with an experimental skill. updatedManifest := testManifest() updatedManifest.Skills["databricks-experimental"] = SkillMeta{ @@ -352,7 +342,7 @@ func TestUpdateSkipsExperimentalSkills(t *testing.T) { Files: []string{"SKILL.md"}, Experimental: true, } - src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + src2 := &mockManifestSource{manifest: updatedManifest} result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) require.NoError(t, err) @@ -372,11 +362,14 @@ func TestUpdateSkipsMinCLIVersionSkills(t *testing.T) { 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} + // Install with default ref. + src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})) + // Simulate a new release. + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.0") + // New manifest with a skill requiring a newer CLI. updatedManifest := testManifest() updatedManifest.Skills["databricks-future"] = SkillMeta{ @@ -384,7 +377,7 @@ func TestUpdateSkipsMinCLIVersionSkills(t *testing.T) { Files: []string{"SKILL.md"}, MinCLIVer: "0.300.0", } - src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0", authoritative: true} + src2 := &mockManifestSource{manifest: updatedManifest} result, err := UpdateSkills(ctx, src2, []*agents.Agent{agent}, UpdateOptions{}) require.NoError(t, err) From 954b49baa8493a9df8632bbf75311ff5bb77fa4b Mon Sep 17 00:00:00 2001 From: simon Date: Thu, 26 Mar 2026 00:15:48 +0100 Subject: [PATCH 7/7] Fix gofmt: remove extra blank line in update_test.go --- experimental/aitools/lib/installer/update_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/experimental/aitools/lib/installer/update_test.go b/experimental/aitools/lib/installer/update_test.go index 862cf4a391..97e3014be6 100644 --- a/experimental/aitools/lib/installer/update_test.go +++ b/experimental/aitools/lib/installer/update_test.go @@ -277,7 +277,6 @@ func TestUpdateOutputSortedAlphabetically(t *testing.T) { assert.Equal(t, "databricks-sql", result.Updated[1].Name) } - func TestUpdateSkillRemovedFromManifestWarning(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context())