From 3e76ad978822a90aa67445377e43117e89dc5df2 Mon Sep 17 00:00:00 2001
From: simon <simon.faltum@databricks.com>
Date: Sun, 22 Mar 2026 08:57:13 +0100
Subject: [PATCH 1/8] Add InstallSkillsForAgents, interactive agent selection,
 idempotent install

Introduces the core InstallSkillsForAgents function that handles:
- Fetching manifest via ManifestSource interface
- Filtering experimental skills and enforcing min_cli_version
- Idempotent install (skips already-installed skills at same version)
- Legacy install detection (skills on disk without state file)
- State persistence after successful install
- Concise output (two lines: installing header + summary)

Adds interactive agent selection in cmd/skills.go using huh multi-select
when multiple agents are detected in an interactive terminal.

Preserves InstallAllSkills signature (func(context.Context) error) for
the cmd/apps/init.go callback.

Co-authored-by: Isaac
---
 experimental/aitools/cmd/install_test.go      | 182 +++++++--
 experimental/aitools/cmd/skills.go            |  74 +++-
 .../aitools/lib/installer/installer.go        | 237 +++++++++---
 .../aitools/lib/installer/installer_test.go   | 366 ++++++++++++++++++
 4 files changed, 763 insertions(+), 96 deletions(-)

diff --git a/experimental/aitools/cmd/install_test.go b/experimental/aitools/cmd/install_test.go
index 64f97ddf9c..8e88f32342 100644
--- a/experimental/aitools/cmd/install_test.go
+++ b/experimental/aitools/cmd/install_test.go
@@ -2,74 +2,178 @@ package aitools
 
 import (
 	"context"
+	"bufio"
+	"os"
+	"path/filepath"
 	"testing"
 
+	"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"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func setupInstallMock(t *testing.T) *[]installCall {
+	t.Helper()
+	orig := installSkillsForAgentsFn
+	t.Cleanup(func() { installSkillsForAgentsFn = orig })
+
+	var calls []installCall
+	installSkillsForAgentsFn = func(_ context.Context, _ installer.ManifestSource, targetAgents []*agents.Agent, opts installer.InstallOptions) error {
+		names := make([]string, len(targetAgents))
+		for i, a := range targetAgents {
+			names[i] = a.Name
+		}
+		calls = append(calls, installCall{agents: names, opts: opts})
+		return nil
+	}
+	return &calls
+}
+
+type installCall struct {
+	agents []string
+	opts   installer.InstallOptions
+}
+
+func setupTestAgents(t *testing.T) string {
+	t.Helper()
+	tmp := t.TempDir()
+	t.Setenv("HOME", tmp)
+	// Create config dirs for two agents.
+	require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".claude"), 0o755))
+	require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".cursor"), 0o755))
+	return tmp
+}
+
 func TestInstallCommandsDelegateToSkillsInstall(t *testing.T) {
-	originalInstallAllSkills := installAllSkills
-	originalInstallSkill := installSkill
-	t.Cleanup(func() {
-		installAllSkills = originalInstallAllSkills
-		installSkill = originalInstallSkill
-	})
+	setupTestAgents(t)
+	calls := setupInstallMock(t)
 
 	tests := []struct {
-		name           string
-		newCmd         func() *cobra.Command
-		args           []string
-		wantAllCalls   int
-		wantSkillCalls []string
+		name       string
+		newCmd     func() *cobra.Command
+		args       []string
+		wantAgents int
+		wantSkills []string
 	}{
 		{
-			name:         "skills install installs all skills",
-			newCmd:       newSkillsInstallCmd,
-			wantAllCalls: 1,
+			name:       "skills install installs all skills for all agents",
+			newCmd:     newSkillsInstallCmd,
+			wantAgents: 2,
 		},
 		{
-			name:           "skills install forwards skill name",
-			newCmd:         newSkillsInstallCmd,
-			args:           []string{"bundle/review"},
-			wantSkillCalls: []string{"bundle/review"},
+			name:       "skills install forwards skill name",
+			newCmd:     newSkillsInstallCmd,
+			args:       []string{"bundle/review"},
+			wantAgents: 2,
+			wantSkills: []string{"bundle/review"},
 		},
 		{
-			name:         "top level install installs all skills",
-			newCmd:       newInstallCmd,
-			wantAllCalls: 1,
+			name:       "top level install installs all skills",
+			newCmd:     newInstallCmd,
+			wantAgents: 2,
 		},
 		{
-			name:           "top level install forwards skill name",
-			newCmd:         newInstallCmd,
-			args:           []string{"bundle/review"},
-			wantSkillCalls: []string{"bundle/review"},
+			name:       "top level install forwards skill name",
+			newCmd:     newInstallCmd,
+			args:       []string{"bundle/review"},
+			wantAgents: 2,
+			wantSkills: []string{"bundle/review"},
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			allCalls := 0
-			var skillCalls []string
-
-			installAllSkills = func(context.Context) error {
-				allCalls++
-				return nil
-			}
-			installSkill = func(_ context.Context, skillName string) error {
-				skillCalls = append(skillCalls, skillName)
-				return nil
-			}
+			*calls = nil
 
+			ctx := cmdio.MockDiscard(t.Context())
 			cmd := tt.newCmd()
-			cmd.SetContext(t.Context())
+			cmd.SetContext(ctx)
 
 			err := cmd.RunE(cmd, tt.args)
 			require.NoError(t, err)
 
-			assert.Equal(t, tt.wantAllCalls, allCalls)
-			assert.Equal(t, tt.wantSkillCalls, skillCalls)
+			require.Len(t, *calls, 1)
+			assert.Len(t, (*calls)[0].agents, tt.wantAgents)
+			assert.Equal(t, tt.wantSkills, (*calls)[0].opts.SpecificSkills)
 		})
 	}
 }
+
+func TestRunSkillsInstallInteractivePrompt(t *testing.T) {
+	setupTestAgents(t)
+	calls := setupInstallMock(t)
+
+	origPrompt := promptAgentSelection
+	t.Cleanup(func() { promptAgentSelection = origPrompt })
+
+	promptCalled := false
+	promptAgentSelection = func(_ context.Context, detected []*agents.Agent) ([]*agents.Agent, error) {
+		promptCalled = true
+		// Return only the first agent.
+		return detected[:1], nil
+	}
+
+	// Use SetupTest with PromptSupported=true to simulate interactive terminal.
+	ctx, test := cmdio.SetupTest(t.Context(), cmdio.TestOptions{PromptSupported: true})
+	defer test.Done()
+
+	// Drain both pipes in background to prevent blocking.
+	drain := func(r *bufio.Reader) {
+		buf := make([]byte, 4096)
+		for {
+			_, err := r.Read(buf)
+			if err != nil {
+				return
+			}
+		}
+	}
+	go drain(test.Stdout)
+	go drain(test.Stderr)
+
+	err := runSkillsInstall(ctx, nil)
+	require.NoError(t, err)
+
+	assert.True(t, promptCalled, "prompt should be called when 2+ agents detected and interactive")
+	require.Len(t, *calls, 1)
+	assert.Len(t, (*calls)[0].agents, 1, "only the selected agent should be passed")
+}
+
+func TestRunSkillsInstallNonInteractiveUsesAllAgents(t *testing.T) {
+	setupTestAgents(t)
+	calls := setupInstallMock(t)
+
+	origPrompt := promptAgentSelection
+	t.Cleanup(func() { promptAgentSelection = origPrompt })
+
+	promptCalled := false
+	promptAgentSelection = func(_ context.Context, detected []*agents.Agent) ([]*agents.Agent, error) {
+		promptCalled = true
+		return detected, nil
+	}
+
+	// MockDiscard gives a non-interactive context.
+	ctx := cmdio.MockDiscard(t.Context())
+
+	err := runSkillsInstall(ctx, nil)
+	require.NoError(t, err)
+
+	assert.False(t, promptCalled, "prompt should not be called in non-interactive mode")
+	require.Len(t, *calls, 1)
+	assert.Len(t, (*calls)[0].agents, 2, "all detected agents should be used")
+}
+
+func TestRunSkillsInstallNoAgents(t *testing.T) {
+	// Set HOME to empty dir so no agents are detected.
+	tmp := t.TempDir()
+	t.Setenv("HOME", tmp)
+
+	calls := setupInstallMock(t)
+	ctx := cmdio.MockDiscard(t.Context())
+
+	err := runSkillsInstall(ctx, nil)
+	require.NoError(t, err)
+	assert.Empty(t, *calls, "install should not be called when no agents detected")
+}
diff --git a/experimental/aitools/cmd/skills.go b/experimental/aitools/cmd/skills.go
index 325ee3c1c0..e29910e57c 100644
--- a/experimental/aitools/cmd/skills.go
+++ b/experimental/aitools/cmd/skills.go
@@ -2,16 +2,51 @@ package aitools
 
 import (
 	"context"
-
+	"fmt"
+	"github.com/charmbracelet/huh"
+	"github.com/databricks/cli/experimental/aitools/lib/agents"
 	"github.com/databricks/cli/experimental/aitools/lib/installer"
+	"github.com/databricks/cli/libs/cmdio"
+	"github.com/fatih/color"
 	"github.com/spf13/cobra"
 )
 
+// Package-level vars for testability.
 var (
-	installAllSkills = installer.InstallAllSkills
-	installSkill     = installer.InstallSkill
+	promptAgentSelection    = defaultPromptAgentSelection
+	installSkillsForAgentsFn = installer.InstallSkillsForAgents
 )
 
+func defaultPromptAgentSelection(ctx context.Context, detected []*agents.Agent) ([]*agents.Agent, error) {
+	options := make([]huh.Option[string], 0, len(detected))
+	agentsByName := make(map[string]*agents.Agent, len(detected))
+	for _, a := range detected {
+		options = append(options, huh.NewOption(a.DisplayName, a.Name).Selected(true))
+		agentsByName[a.Name] = a
+	}
+
+	var selected []string
+	err := huh.NewMultiSelect[string]().
+		Title("Select coding agents to install skills for").
+		Description("space to toggle, enter to confirm").
+		Options(options...).
+		Value(&selected).
+		Run()
+	if err != nil {
+		return nil, err
+	}
+
+	if len(selected) == 0 {
+		return nil, fmt.Errorf("at least one agent must be selected")
+	}
+
+	result := make([]*agents.Agent, 0, len(selected))
+	for _, name := range selected {
+		result = append(result, agentsByName[name])
+	}
+	return result, nil
+}
+
 func newSkillsCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "skills",
@@ -53,9 +88,38 @@ Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Anti
 }
 
 func runSkillsInstall(ctx context.Context, args []string) error {
+	detected := agents.DetectInstalled(ctx)
+	if len(detected) == 0 {
+		cmdio.LogString(ctx, color.YellowString("No supported coding agents detected."))
+		cmdio.LogString(ctx, "")
+		cmdio.LogString(ctx, "Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity")
+		cmdio.LogString(ctx, "Please install at least one coding agent first.")
+		return nil
+	}
+
+	var targetAgents []*agents.Agent
+	switch {
+	case len(detected) == 1:
+		targetAgents = detected
+	case cmdio.IsPromptSupported(ctx):
+		var err error
+		targetAgents, err = promptAgentSelection(ctx, detected)
+		if err != nil {
+			return err
+		}
+	default:
+		// Non-interactive: install for all detected agents.
+		targetAgents = detected
+	}
+
+	installer.PrintInstallingFor(ctx, targetAgents)
+
+	opts := installer.InstallOptions{}
 	if len(args) > 0 {
-		return installSkill(ctx, args[0])
+		opts.SpecificSkills = []string{args[0]}
 	}
 
-	return installAllSkills(ctx)
+	src := &installer.GitHubManifestSource{}
+	return installSkillsForAgentsFn(ctx, src, targetAgents, opts)
 }
+
diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go
index b9f2d7b899..18e8c6f1e0 100644
--- a/experimental/aitools/lib/installer/installer.go
+++ b/experimental/aitools/lib/installer/installer.go
@@ -7,12 +7,16 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"github.com/databricks/cli/experimental/aitools/lib/agents"
+	"github.com/databricks/cli/internal/build"
 	"github.com/databricks/cli/libs/cmdio"
 	"github.com/databricks/cli/libs/env"
+	"github.com/databricks/cli/libs/log"
 	"github.com/fatih/color"
+	"golang.org/x/mod/semver"
 )
 
 const (
@@ -22,6 +26,10 @@ const (
 	defaultSkillsRepoRef = "v0.1.3"
 )
 
+// fetchFileFn is the function used to download individual skill files.
+// It is a package-level var so tests can replace it with a mock.
+var fetchFileFn = fetchSkillFile
+
 func getSkillsRef(ctx context.Context) string {
 	if ref := env.Get(ctx, "DATABRICKS_SKILLS_REF"); ref != "" {
 		return ref
@@ -46,6 +54,12 @@ type SkillMeta struct {
 	MinCLIVer    string   `json:"min_cli_version,omitempty"`
 }
 
+// InstallOptions controls the behavior of InstallSkillsForAgents.
+type InstallOptions struct {
+	IncludeExperimental bool
+	SpecificSkills      []string // empty = all skills
+}
+
 // FetchManifest fetches the skills manifest from the skills repo.
 // This is a convenience wrapper that uses the default GitHubManifestSource.
 func FetchManifest(ctx context.Context) (*Manifest, error) {
@@ -54,9 +68,9 @@ func FetchManifest(ctx context.Context) (*Manifest, error) {
 	return src.FetchManifest(ctx, ref)
 }
 
-func fetchSkillFile(ctx context.Context, skillName, filePath string) ([]byte, error) {
+func fetchSkillFile(ctx context.Context, ref, skillName, filePath string) ([]byte, error) {
 	url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s/%s/%s",
-		skillsRepoOwner, skillsRepoName, getSkillsRef(ctx), skillsRepoPath, skillName, filePath)
+		skillsRepoOwner, skillsRepoName, ref, skillsRepoPath, skillName, filePath)
 
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
 	if err != nil {
@@ -97,49 +111,157 @@ func ListSkills(ctx context.Context) error {
 	return nil
 }
 
-// InstallAllSkills fetches the manifest and installs all skills for detected agents.
-func InstallAllSkills(ctx context.Context) error {
-	manifest, err := FetchManifest(ctx)
+// InstallSkillsForAgents fetches the manifest and installs skills for the given agents.
+// This is the core installation function. Callers are responsible for agent detection,
+// prompting, and printing the "Installing..." header.
+func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgents []*agents.Agent, opts InstallOptions) error {
+	latestTag, err := src.FetchLatestRelease(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to fetch latest release: %w", err)
+	}
+
+	manifest, err := src.FetchManifest(ctx, latestTag)
 	if err != nil {
 		return err
 	}
 
-	detectedAgents := agents.DetectInstalled(ctx)
-	if len(detectedAgents) == 0 {
-		printNoAgentsDetected(ctx)
-		return nil
+	globalDir, err := GlobalSkillsDir(ctx)
+	if err != nil {
+		return err
 	}
 
-	printDetectedAgents(ctx, detectedAgents)
+	// Load existing state for idempotency checks.
+	state, err := LoadState(globalDir)
+	if err != nil {
+		return fmt.Errorf("failed to load install state: %w", err)
+	}
 
-	for name, meta := range manifest.Skills {
-		if err := installSkillForAgents(ctx, name, meta.Files, detectedAgents); err != nil {
+	// Detect legacy installs (skills on disk but no state file).
+	if state == nil {
+		checkLegacyInstall(ctx, globalDir)
+	}
+
+	// Filter skills based on options, experimental flag, and CLI version.
+	targetSkills, err := resolveSkills(ctx, manifest.Skills, opts)
+	if err != nil {
+		return err
+	}
+
+	// Install each skill.
+	for name, meta := range targetSkills {
+		// Idempotency: skip if same version is already installed and on disk.
+		if state != nil && state.Skills[name] == meta.Version {
+			skillDir := filepath.Join(globalDir, name)
+			if _, statErr := os.Stat(skillDir); statErr == nil {
+				log.Debugf(ctx, "%s v%s already installed, skipping", name, meta.Version)
+				continue
+			}
+		}
+
+		if err := installSkillForAgents(ctx, latestTag, name, meta.Files, targetAgents, globalDir); err != nil {
 			return err
 		}
 	}
+
+	// Save state.
+	newState := &InstallState{
+		SchemaVersion:       1,
+		IncludeExperimental: opts.IncludeExperimental,
+		Release:             latestTag,
+		LastUpdated:         time.Now(),
+		Skills:              make(map[string]string, len(targetSkills)),
+	}
+	for name, meta := range targetSkills {
+		newState.Skills[name] = meta.Version
+	}
+	if err := SaveState(globalDir, newState); err != nil {
+		return err
+	}
+
+	tag := strings.TrimPrefix(latestTag, "v")
+	cmdio.LogString(ctx, fmt.Sprintf("Installed %d skills (v%s).", len(targetSkills), tag))
 	return nil
 }
 
-// InstallSkill fetches the manifest and installs a single skill by name.
-func InstallSkill(ctx context.Context, skillName string) error {
-	manifest, err := FetchManifest(ctx)
-	if err != nil {
-		return err
+// resolveSkills filters the manifest skills based on the install options,
+// experimental flag, and CLI version constraints.
+func resolveSkills(ctx context.Context, skills map[string]SkillMeta, opts InstallOptions) (map[string]SkillMeta, error) {
+	isSpecific := len(opts.SpecificSkills) > 0
+	cliVersion := build.GetInfo().Version
+	isDev := strings.HasPrefix(cliVersion, build.DefaultSemver)
+
+	// Start with all skills or only the requested ones.
+	var candidates map[string]SkillMeta
+	if isSpecific {
+		candidates = make(map[string]SkillMeta, len(opts.SpecificSkills))
+		for _, name := range opts.SpecificSkills {
+			meta, ok := skills[name]
+			if !ok {
+				return nil, fmt.Errorf("skill %q not found", name)
+			}
+			candidates[name] = meta
+		}
+	} else {
+		candidates = skills
 	}
 
-	if _, ok := manifest.Skills[skillName]; !ok {
-		return fmt.Errorf("skill %q not found", skillName)
+	result := make(map[string]SkillMeta, len(candidates))
+	for name, meta := range candidates {
+		if meta.Experimental && !opts.IncludeExperimental {
+			if isSpecific {
+				return nil, fmt.Errorf("skill %q is experimental; use --experimental to install", name)
+			}
+			log.Debugf(ctx, "Skipping experimental skill %s", name)
+			continue
+		}
+
+		if meta.MinCLIVer != "" && !isDev && semver.Compare("v"+cliVersion, "v"+meta.MinCLIVer) < 0 {
+			if isSpecific {
+				return nil, fmt.Errorf("skill %q requires CLI version %s (running %s)", name, meta.MinCLIVer, cliVersion)
+			}
+			log.Warnf(ctx, "Skipping %s: requires CLI version %s (running %s)", name, meta.MinCLIVer, cliVersion)
+			continue
+		}
+
+		result[name] = meta
 	}
+	return result, nil
+}
 
-	detectedAgents := agents.DetectInstalled(ctx)
-	if len(detectedAgents) == 0 {
+// InstallAllSkills fetches the manifest and installs all skills for detected agents.
+// The signature is func(context.Context) error to satisfy the callback in cmd/apps/init.go.
+func InstallAllSkills(ctx context.Context) error {
+	installed := agents.DetectInstalled(ctx)
+	if len(installed) == 0 {
 		printNoAgentsDetected(ctx)
 		return nil
 	}
 
-	printDetectedAgents(ctx, detectedAgents)
+	PrintInstallingFor(ctx, installed)
+	src := &GitHubManifestSource{}
+	return InstallSkillsForAgents(ctx, src, installed, InstallOptions{})
+}
 
-	return installSkillForAgents(ctx, skillName, manifest.Skills[skillName].Files, detectedAgents)
+// InstallSkill installs a single skill by name for all detected agents.
+func InstallSkill(ctx context.Context, skillName string) error {
+	installed := agents.DetectInstalled(ctx)
+	if len(installed) == 0 {
+		printNoAgentsDetected(ctx)
+		return nil
+	}
+
+	PrintInstallingFor(ctx, installed)
+	src := &GitHubManifestSource{}
+	return InstallSkillsForAgents(ctx, src, installed, InstallOptions{SpecificSkills: []string{skillName}})
+}
+
+// PrintInstallingFor prints the "Installing..." header with agent names.
+func PrintInstallingFor(ctx context.Context, targetAgents []*agents.Agent) {
+	names := make([]string, len(targetAgents))
+	for i, a := range targetAgents {
+		names[i] = a.DisplayName
+	}
+	cmdio.LogString(ctx, fmt.Sprintf("Installing Databricks AI skills for %s...", strings.Join(names, ", ")))
 }
 
 func printNoAgentsDetected(ctx context.Context) {
@@ -149,61 +271,73 @@ func printNoAgentsDetected(ctx context.Context) {
 	cmdio.LogString(ctx, "Please install at least one coding agent first.")
 }
 
-func printDetectedAgents(ctx context.Context, detectedAgents []*agents.Agent) {
-	cmdio.LogString(ctx, "Detected coding agents:")
-	for _, agent := range detectedAgents {
-		cmdio.LogString(ctx, "  - "+agent.DisplayName)
+// checkLegacyInstall prints a message if skills exist on disk but no state file was found.
+func checkLegacyInstall(ctx context.Context, globalDir string) {
+	if hasSkillsOnDisk(globalDir) {
+		cmdio.LogString(ctx, "Found skills installed before state tracking was added. Run 'databricks experimental aitools install' to refresh.")
+		return
+	}
+	homeDir, err := env.UserHomeDir(ctx)
+	if err != nil {
+		return
+	}
+	legacyDir := filepath.Join(homeDir, ".databricks", "agent-skills")
+	if hasSkillsOnDisk(legacyDir) {
+		cmdio.LogString(ctx, "Found skills installed before state tracking was added. Run 'databricks experimental aitools install' to refresh.")
 	}
-	cmdio.LogString(ctx, "")
 }
 
-func installSkillForAgents(ctx context.Context, skillName string, files []string, detectedAgents []*agents.Agent) error {
-	homeDir, err := env.UserHomeDir(ctx)
+// hasSkillsOnDisk checks if a directory contains subdirectories starting with "databricks".
+func hasSkillsOnDisk(dir string) bool {
+	entries, err := os.ReadDir(dir)
 	if err != nil {
-		return fmt.Errorf("failed to get home directory: %w", err)
+		return false
+	}
+	for _, e := range entries {
+		if e.IsDir() && strings.HasPrefix(e.Name(), "databricks") {
+			return true
+		}
 	}
+	return false
+}
 
-	// Always install to canonical location first.
-	canonicalDir := filepath.Join(homeDir, agents.CanonicalSkillsDir, skillName)
-	if err := installSkillToDir(ctx, skillName, canonicalDir, files); err != nil {
+func installSkillForAgents(ctx context.Context, ref, skillName string, files []string, detectedAgents []*agents.Agent, globalDir string) error {
+	canonicalDir := filepath.Join(globalDir, skillName)
+	if err := installSkillToDir(ctx, ref, skillName, canonicalDir, files); err != nil {
 		return err
 	}
 
 	useSymlinks := len(detectedAgents) > 1
 
-	// install/symlink to each agent
 	for _, agent := range detectedAgents {
 		agentSkillDir, err := agent.SkillsDir(ctx)
 		if err != nil {
-			cmdio.LogString(ctx, color.YellowString("⊘ Skipped %s: %v", agent.DisplayName, err))
+			log.Warnf(ctx, "Skipped %s: %v", agent.DisplayName, err)
 			continue
 		}
 
 		destDir := filepath.Join(agentSkillDir, skillName)
 
-		// Back up existing non-canonical skills before overwriting.
 		if err := backupThirdPartySkill(ctx, destDir, canonicalDir, skillName, agent.DisplayName); err != nil {
-			cmdio.LogString(ctx, color.YellowString("⊘ Failed to back up existing skill for %s: %v", agent.DisplayName, err))
+			log.Warnf(ctx, "Failed to back up existing skill for %s: %v", agent.DisplayName, err)
 			continue
 		}
 
 		if useSymlinks {
 			if err := createSymlink(canonicalDir, destDir); err != nil {
-				// fallback to copy on symlink failure (e.g., Windows without admin)
-				cmdio.LogString(ctx, color.YellowString("  Symlink failed for %s, copying instead...", agent.DisplayName))
-				if err := installSkillToDir(ctx, skillName, destDir, files); err != nil {
-					cmdio.LogString(ctx, color.YellowString("⊘ Failed to install for %s: %v", agent.DisplayName, err))
+				log.Debugf(ctx, "Symlink failed for %s, copying instead: %v", agent.DisplayName, err)
+				if err := installSkillToDir(ctx, ref, skillName, destDir, files); err != nil {
+					log.Warnf(ctx, "Failed to install for %s: %v", agent.DisplayName, err)
 					continue
 				}
 			}
-			cmdio.LogString(ctx, color.GreenString("✓ Installed %q for %s (symlinked)", skillName, agent.DisplayName))
+			log.Debugf(ctx, "Installed %q for %s (symlinked)", skillName, agent.DisplayName)
 		} else {
-			// single agent - copy from canonical
-			if err := installSkillToDir(ctx, skillName, destDir, files); err != nil {
-				cmdio.LogString(ctx, color.YellowString("⊘ Failed to install for %s: %v", agent.DisplayName, err))
+			if err := installSkillToDir(ctx, ref, skillName, destDir, files); err != nil {
+				log.Warnf(ctx, "Failed to install for %s: %v", agent.DisplayName, err)
 				continue
 			}
-			cmdio.LogString(ctx, color.GreenString("✓ Installed %q for %s", skillName, agent.DisplayName))
+			log.Debugf(ctx, "Installed %q for %s", skillName, agent.DisplayName)
 		}
 	}
 
@@ -239,11 +373,11 @@ func backupThirdPartySkill(ctx context.Context, destDir, canonicalDir, skillName
 		return fmt.Errorf("failed to move existing skill: %w", err)
 	}
 
-	cmdio.LogString(ctx, color.YellowString("  Existing %q for %s moved to %s", skillName, agentName, backupDest))
+	log.Debugf(ctx, "Existing %q for %s moved to %s", skillName, agentName, backupDest)
 	return nil
 }
 
-func installSkillToDir(ctx context.Context, skillName, destDir string, files []string) error {
+func installSkillToDir(ctx context.Context, ref, skillName, destDir string, files []string) error {
 	// remove existing skill directory for clean install
 	if err := os.RemoveAll(destDir); err != nil {
 		return fmt.Errorf("failed to remove existing skill: %w", err)
@@ -253,20 +387,19 @@ func installSkillToDir(ctx context.Context, skillName, destDir string, files []s
 		return fmt.Errorf("failed to create directory: %w", err)
 	}
 
-	// download all files
 	for _, file := range files {
-		content, err := fetchSkillFile(ctx, skillName, file)
+		content, err := fetchFileFn(ctx, ref, skillName, file)
 		if err != nil {
 			return err
 		}
 
 		destPath := filepath.Join(destDir, file)
 
-		// create parent directories if needed
 		if err := os.MkdirAll(filepath.Dir(destPath), 0o755); err != nil {
 			return fmt.Errorf("failed to create directory: %w", err)
 		}
 
+		log.Debugf(ctx, "Downloading %s/%s", skillName, file)
 		if err := os.WriteFile(destPath, content, 0o644); err != nil {
 			return fmt.Errorf("failed to write %s: %w", file, err)
 		}
diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go
index 5caa1e3b57..1636c0ff30 100644
--- a/experimental/aitools/lib/installer/installer_test.go
+++ b/experimental/aitools/lib/installer/installer_test.go
@@ -1,15 +1,90 @@
 package installer
 
 import (
+	"context"
 	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/databricks/cli/experimental/aitools/lib/agents"
+	"github.com/databricks/cli/internal/build"
 	"github.com/databricks/cli/libs/cmdio"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+// mockManifestSource is a test double for ManifestSource.
+type mockManifestSource struct {
+	manifest *Manifest
+	release  string
+	fetchErr error
+}
+
+func (m *mockManifestSource) FetchManifest(_ context.Context, _ string) (*Manifest, error) {
+	if m.fetchErr != nil {
+		return nil, m.fetchErr
+	}
+	return m.manifest, nil
+}
+
+func (m *mockManifestSource) FetchLatestRelease(_ context.Context) (string, error) {
+	return m.release, nil
+}
+
+func testManifest() *Manifest {
+	return &Manifest{
+		Version:   "1",
+		UpdatedAt: "2024-01-01",
+		Skills: map[string]SkillMeta{
+			"databricks-sql": {
+				Version: "0.1.0",
+				Files:   []string{"SKILL.md"},
+			},
+			"databricks-jobs": {
+				Version: "0.1.0",
+				Files:   []string{"SKILL.md"},
+			},
+		},
+	}
+}
+
+func setupFetchMock(t *testing.T) {
+	t.Helper()
+	orig := fetchFileFn
+	t.Cleanup(func() { fetchFileFn = orig })
+	fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) {
+		return []byte("# " + skillName + "/" + filePath), nil
+	}
+}
+
+func testAgent(tmpHome string) *agents.Agent {
+	return &agents.Agent{
+		Name:        "test-agent",
+		DisplayName: "Test Agent",
+		ConfigDir: func(_ context.Context) (string, error) {
+			return filepath.Join(tmpHome, ".test-agent"), nil
+		},
+	}
+}
+
+func setupTestHome(t *testing.T) string {
+	t.Helper()
+	tmp := t.TempDir()
+	t.Setenv("HOME", tmp)
+	// Create agent config dir so the agent is "detected".
+	require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".test-agent"), 0o755))
+	return tmp
+}
+
+func setBuildVersion(t *testing.T, version string) {
+	t.Helper()
+	orig := build.GetInfo().Version
+	build.SetBuildVersion(version)
+	t.Cleanup(func() { build.SetBuildVersion(orig) })
+}
+
+// --- Backup tests (unchanged from PR 1) ---
+
 func TestBackupThirdPartySkillDestDoesNotExist(t *testing.T) {
 	ctx := cmdio.MockDiscard(t.Context())
 	destDir := filepath.Join(t.TempDir(), "nonexistent")
@@ -109,3 +184,294 @@ func TestBackupThirdPartySkillRegularFile(t *testing.T) {
 	_, err = os.Stat(destDir)
 	assert.True(t, os.IsNotExist(err))
 }
+
+// --- InstallSkillsForAgents tests ---
+
+func TestInstallSkillsForAgentsWritesState(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
+	setupFetchMock(t)
+
+	src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
+	require.NoError(t, err)
+
+	globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
+	state, err := LoadState(globalDir)
+	require.NoError(t, err)
+	require.NotNil(t, state)
+	assert.Equal(t, 1, state.SchemaVersion)
+	assert.Equal(t, "v0.1.0", state.Release)
+	assert.Len(t, state.Skills, 2)
+	assert.Equal(t, "0.1.0", state.Skills["databricks-sql"])
+	assert.Equal(t, "0.1.0", state.Skills["databricks-jobs"])
+
+	assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.0).")
+}
+
+func TestInstallSkillForSingleWritesState(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
+	setupFetchMock(t)
+
+	src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{
+		SpecificSkills: []string{"databricks-sql"},
+	})
+	require.NoError(t, err)
+
+	globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
+	state, err := LoadState(globalDir)
+	require.NoError(t, err)
+	require.NotNil(t, state)
+	assert.Len(t, state.Skills, 1)
+	assert.Equal(t, "0.1.0", state.Skills["databricks-sql"])
+
+	assert.Contains(t, stderr.String(), "Installed 1 skills (v0.1.0).")
+}
+
+func TestInstallSkillsSpecificNotFound(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx := cmdio.MockDiscard(t.Context())
+	setupFetchMock(t)
+
+	src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{
+		SpecificSkills: []string{"nonexistent"},
+	})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), `skill "nonexistent" not found`)
+}
+
+func TestExperimentalSkillsSkippedByDefault(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
+	setupFetchMock(t)
+
+	manifest := testManifest()
+	manifest.Skills["databricks-experimental"] = SkillMeta{
+		Version:      "0.1.0",
+		Files:        []string{"SKILL.md"},
+		Experimental: true,
+	}
+
+	src := &mockManifestSource{manifest: manifest, release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
+	require.NoError(t, err)
+
+	globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
+	state, err := LoadState(globalDir)
+	require.NoError(t, err)
+	// Only non-experimental skills should be installed.
+	assert.Len(t, state.Skills, 2)
+	assert.NotContains(t, state.Skills, "databricks-experimental")
+
+	assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.0).")
+}
+
+func TestExperimentalSkillsIncludedWithFlag(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
+	setupFetchMock(t)
+
+	manifest := testManifest()
+	manifest.Skills["databricks-experimental"] = SkillMeta{
+		Version:      "0.1.0",
+		Files:        []string{"SKILL.md"},
+		Experimental: true,
+	}
+
+	src := &mockManifestSource{manifest: manifest, release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{
+		IncludeExperimental: true,
+	})
+	require.NoError(t, err)
+
+	globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
+	state, err := LoadState(globalDir)
+	require.NoError(t, err)
+	assert.Len(t, state.Skills, 3)
+	assert.Contains(t, state.Skills, "databricks-experimental")
+	assert.True(t, state.IncludeExperimental)
+
+	assert.Contains(t, stderr.String(), "Installed 3 skills (v0.1.0).")
+}
+
+func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
+	setupFetchMock(t)
+	setBuildVersion(t, "0.200.0")
+
+	manifest := testManifest()
+	manifest.Skills["databricks-future"] = SkillMeta{
+		Version:   "0.1.0",
+		Files:     []string{"SKILL.md"},
+		MinCLIVer: "0.300.0",
+	}
+
+	src := &mockManifestSource{manifest: manifest, release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
+	require.NoError(t, err)
+
+	globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
+	state, err := LoadState(globalDir)
+	require.NoError(t, err)
+	// The high-version skill should be skipped.
+	assert.Len(t, state.Skills, 2)
+	assert.NotContains(t, state.Skills, "databricks-future")
+
+	assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.0).")
+}
+
+func TestMinCLIVersionHardErrorForInstallSingle(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx := cmdio.MockDiscard(t.Context())
+	setupFetchMock(t)
+	setBuildVersion(t, "0.200.0")
+
+	manifest := testManifest()
+	manifest.Skills["databricks-future"] = SkillMeta{
+		Version:   "0.1.0",
+		Files:     []string{"SKILL.md"},
+		MinCLIVer: "0.300.0",
+	}
+
+	src := &mockManifestSource{manifest: manifest, release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{
+		SpecificSkills: []string{"databricks-future"},
+	})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "requires CLI version 0.300.0")
+	assert.Contains(t, err.Error(), "running 0.200.0")
+}
+
+func TestIdempotentSecondInstallSkips(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx := cmdio.MockDiscard(t.Context())
+	setupFetchMock(t)
+
+	src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	// First install.
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
+	require.NoError(t, err)
+
+	// Track fetch calls on second install.
+	fetchCalls := 0
+	orig := fetchFileFn
+	t.Cleanup(func() { fetchFileFn = orig })
+	fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) {
+		fetchCalls++
+		return []byte("# " + skillName + "/" + filePath), nil
+	}
+
+	// Second install with same version.
+	err = InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
+	require.NoError(t, err)
+
+	// No files should be fetched since everything is up to date.
+	assert.Equal(t, 0, fetchCalls)
+}
+
+func TestIdempotentInstallUpdatesNewVersions(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx := cmdio.MockDiscard(t.Context())
+	setupFetchMock(t)
+
+	src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	// First install.
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
+	require.NoError(t, err)
+
+	// Update manifest with a new version for one skill.
+	updatedManifest := testManifest()
+	updatedManifest.Skills["databricks-sql"] = SkillMeta{
+		Version: "0.2.0",
+		Files:   []string{"SKILL.md"},
+	}
+	src2 := &mockManifestSource{manifest: updatedManifest, release: "v0.2.0"}
+
+	// Track which skills are fetched.
+	var fetchedSkills []string
+	orig := fetchFileFn
+	t.Cleanup(func() { fetchFileFn = orig })
+	fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) {
+		fetchedSkills = append(fetchedSkills, skillName)
+		return []byte("# " + skillName + "/" + filePath), nil
+	}
+
+	// Second install with updated manifest.
+	err = InstallSkillsForAgents(ctx, src2, []*agents.Agent{agent}, InstallOptions{})
+	require.NoError(t, err)
+
+	// Both skills should be fetched because the release tag changed.
+	// (databricks-sql has a new version, databricks-jobs has the same version
+	// but state was from v0.1.0 release.)
+	assert.Contains(t, fetchedSkills, "databricks-sql")
+
+	globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
+	state, err := LoadState(globalDir)
+	require.NoError(t, err)
+	assert.Equal(t, "v0.2.0", state.Release)
+	assert.Equal(t, "0.2.0", state.Skills["databricks-sql"])
+}
+
+func TestLegacyDetectMessagePrinted(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
+	setupFetchMock(t)
+
+	// Create skills on disk at canonical location but no state file.
+	globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
+	require.NoError(t, os.MkdirAll(filepath.Join(globalDir, "databricks-sql"), 0o755))
+
+	src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
+	require.NoError(t, err)
+
+	assert.Contains(t, stderr.String(), "Found skills installed before state tracking was added.")
+}
+
+func TestLegacyDetectLegacyDir(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
+	setupFetchMock(t)
+
+	// Create skills in the legacy location.
+	legacyDir := filepath.Join(tmp, ".databricks", "agent-skills")
+	require.NoError(t, os.MkdirAll(filepath.Join(legacyDir, "databricks-sql"), 0o755))
+
+	src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"}
+	agent := testAgent(tmp)
+
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
+	require.NoError(t, err)
+
+	assert.Contains(t, stderr.String(), "Found skills installed before state tracking was added.")
+}
+
+func TestInstallAllSkillsSignaturePreserved(t *testing.T) {
+	// Compile-time check that InstallAllSkills satisfies func(context.Context) error.
+	var fn func(context.Context) error = InstallAllSkills
+	_ = fn
+}

From 8c5d4926c99e0fa999c83fccf7a45a7c4d824b97 Mon Sep 17 00:00:00 2001
From: simon <simon.faltum@databricks.com>
Date: Sun, 22 Mar 2026 09:07:44 +0100
Subject: [PATCH 2/8] Fix review findings: flag name, state merge, log level,
 grammar, test assertion

- Fix --experimental -> --include-experimental in error message
- Merge new skills into existing state instead of overwriting
- Move FetchManifest log from Info to Debug for concise output
- Handle singular/plural in Installed N skill(s) message
- Assert min_cli_version skip warning appears in test output

Co-authored-by: Isaac
---
 .../aitools/lib/installer/installer.go        | 41 +++++++++++++------
 .../aitools/lib/installer/installer_test.go   | 11 ++++-
 experimental/aitools/lib/installer/source.go  |  2 +-
 3 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go
index 18e8c6f1e0..ab1decba2c 100644
--- a/experimental/aitools/lib/installer/installer.go
+++ b/experimental/aitools/lib/installer/installer.go
@@ -163,23 +163,40 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 		}
 	}
 
-	// Save state.
-	newState := &InstallState{
-		SchemaVersion:       1,
-		IncludeExperimental: opts.IncludeExperimental,
-		Release:             latestTag,
-		LastUpdated:         time.Now(),
-		Skills:              make(map[string]string, len(targetSkills)),
-	}
-	for name, meta := range targetSkills {
-		newState.Skills[name] = meta.Version
+	// Save state. Merge into existing state so skills from previous installs
+	// (e.g., experimental skills from a prior run) are preserved.
+	existingState, _ := LoadState(globalDir)
+	var newState *InstallState
+	if existingState != nil {
+		newState = existingState
+		newState.Release = latestTag
+		newState.LastUpdated = time.Now()
+		newState.IncludeExperimental = opts.IncludeExperimental
+		for name, meta := range targetSkills {
+			newState.Skills[name] = meta.Version
+		}
+	} else {
+		newState = &InstallState{
+			SchemaVersion:       1,
+			IncludeExperimental: opts.IncludeExperimental,
+			Release:             latestTag,
+			LastUpdated:         time.Now(),
+			Skills:              make(map[string]string, len(targetSkills)),
+		}
+		for name, meta := range targetSkills {
+			newState.Skills[name] = meta.Version
+		}
 	}
 	if err := SaveState(globalDir, newState); err != nil {
 		return err
 	}
 
 	tag := strings.TrimPrefix(latestTag, "v")
-	cmdio.LogString(ctx, fmt.Sprintf("Installed %d skills (v%s).", len(targetSkills), tag))
+	noun := "skills"
+	if len(targetSkills) == 1 {
+		noun = "skill"
+	}
+	cmdio.LogString(ctx, fmt.Sprintf("Installed %d %s (v%s).", len(targetSkills), noun, tag))
 	return nil
 }
 
@@ -209,7 +226,7 @@ func resolveSkills(ctx context.Context, skills map[string]SkillMeta, opts Instal
 	for name, meta := range candidates {
 		if meta.Experimental && !opts.IncludeExperimental {
 			if isSpecific {
-				return nil, fmt.Errorf("skill %q is experimental; use --experimental to install", name)
+				return nil, fmt.Errorf("skill %q is experimental; use --include-experimental to install", name)
 			}
 			log.Debugf(ctx, "Skipping experimental skill %s", name)
 			continue
diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go
index 1636c0ff30..7d9ebc95a3 100644
--- a/experimental/aitools/lib/installer/installer_test.go
+++ b/experimental/aitools/lib/installer/installer_test.go
@@ -1,7 +1,9 @@
 package installer
 
 import (
+	"bytes"
 	"context"
+	"log/slog"
 	"os"
 	"path/filepath"
 	"testing"
@@ -9,6 +11,7 @@ import (
 	"github.com/databricks/cli/experimental/aitools/lib/agents"
 	"github.com/databricks/cli/internal/build"
 	"github.com/databricks/cli/libs/cmdio"
+	"github.com/databricks/cli/libs/log"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -231,7 +234,7 @@ func TestInstallSkillForSingleWritesState(t *testing.T) {
 	assert.Len(t, state.Skills, 1)
 	assert.Equal(t, "0.1.0", state.Skills["databricks-sql"])
 
-	assert.Contains(t, stderr.String(), "Installed 1 skills (v0.1.0).")
+	assert.Contains(t, stderr.String(), "Installed 1 skill (v0.1.0).")
 }
 
 func TestInstallSkillsSpecificNotFound(t *testing.T) {
@@ -313,6 +316,11 @@ func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) {
 	setupFetchMock(t)
 	setBuildVersion(t, "0.200.0")
 
+	// Capture log output to verify the warning.
+	var logBuf bytes.Buffer
+	logger := slog.New(slog.NewTextHandler(&logBuf, &slog.HandlerOptions{Level: slog.LevelWarn}))
+	ctx = log.NewContext(ctx, logger)
+
 	manifest := testManifest()
 	manifest.Skills["databricks-future"] = SkillMeta{
 		Version:   "0.1.0",
@@ -334,6 +342,7 @@ func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) {
 	assert.NotContains(t, state.Skills, "databricks-future")
 
 	assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.0).")
+	assert.Contains(t, logBuf.String(), "requires CLI version 0.300.0")
 }
 
 func TestMinCLIVersionHardErrorForInstallSingle(t *testing.T) {
diff --git a/experimental/aitools/lib/installer/source.go b/experimental/aitools/lib/installer/source.go
index 86984efa53..e601b26d66 100644
--- a/experimental/aitools/lib/installer/source.go
+++ b/experimental/aitools/lib/installer/source.go
@@ -21,7 +21,7 @@ type GitHubManifestSource struct{}
 
 // FetchManifest fetches the skills manifest from GitHub at the given ref.
 func (s *GitHubManifestSource) FetchManifest(ctx context.Context, ref string) (*Manifest, error) {
-	log.Infof(ctx, "Fetching skills manifest from %s/%s@%s", skillsRepoOwner, skillsRepoName, ref)
+	log.Debugf(ctx, "Fetching skills manifest from %s/%s@%s", skillsRepoOwner, skillsRepoName, ref)
 	url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/manifest.json",
 		skillsRepoOwner, skillsRepoName, ref)
 

From 3fdf9b3ed9070964b2e34c3346fade71f35b9c90 Mon Sep 17 00:00:00 2001
From: simon <simon.faltum@databricks.com>
Date: Sun, 22 Mar 2026 09:08:37 +0100
Subject: [PATCH 3/8] Fix trailing whitespace in skills.go

---
 experimental/aitools/cmd/skills.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/experimental/aitools/cmd/skills.go b/experimental/aitools/cmd/skills.go
index e29910e57c..9550eb6b33 100644
--- a/experimental/aitools/cmd/skills.go
+++ b/experimental/aitools/cmd/skills.go
@@ -122,4 +122,3 @@ func runSkillsInstall(ctx context.Context, args []string) error {
 	src := &installer.GitHubManifestSource{}
 	return installSkillsForAgentsFn(ctx, src, targetAgents, opts)
 }
-

From 7ba86c1b3a87b023e99fbb71d3b492c1979a544c Mon Sep 17 00:00:00 2001
From: simon <simon.faltum@databricks.com>
Date: Sun, 22 Mar 2026 11:37:50 +0100
Subject: [PATCH 4/8] Sort install order, log state reload errors

Co-authored-by: Isaac
---
 experimental/aitools/lib/installer/installer.go | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go
index ab1decba2c..2af50222ea 100644
--- a/experimental/aitools/lib/installer/installer.go
+++ b/experimental/aitools/lib/installer/installer.go
@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"time"
 
@@ -147,8 +148,15 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 		return err
 	}
 
-	// Install each skill.
-	for name, meta := range targetSkills {
+	// Install each skill in sorted order for determinism.
+	skillNames := make([]string, 0, len(targetSkills))
+	for name := range targetSkills {
+		skillNames = append(skillNames, name)
+	}
+	sort.Strings(skillNames)
+
+	for _, name := range skillNames {
+		meta := targetSkills[name]
 		// Idempotency: skip if same version is already installed and on disk.
 		if state != nil && state.Skills[name] == meta.Version {
 			skillDir := filepath.Join(globalDir, name)
@@ -165,7 +173,10 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 
 	// Save state. Merge into existing state so skills from previous installs
 	// (e.g., experimental skills from a prior run) are preserved.
-	existingState, _ := LoadState(globalDir)
+	existingState, loadErr := LoadState(globalDir)
+	if loadErr != nil {
+		log.Warnf(ctx, "Could not reload state for merge: %v", loadErr)
+	}
 	var newState *InstallState
 	if existingState != nil {
 		newState = existingState

From 7461ae7b89a3811908af25d4dd64ab96acfcf48f Mon Sep 17 00:00:00 2001
From: simon <simon.faltum@databricks.com>
Date: Sun, 22 Mar 2026 22:54:24 +0100
Subject: [PATCH 5/8] Fix gofmt, gofumpt, perfsprint, and staticcheck lint
 issues

Co-authored-by: Isaac
---
 experimental/aitools/cmd/install_test.go             | 2 +-
 experimental/aitools/cmd/skills.go                   | 7 ++++---
 experimental/aitools/lib/installer/installer_test.go | 4 ++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/experimental/aitools/cmd/install_test.go b/experimental/aitools/cmd/install_test.go
index 8e88f32342..80a19317b1 100644
--- a/experimental/aitools/cmd/install_test.go
+++ b/experimental/aitools/cmd/install_test.go
@@ -1,8 +1,8 @@
 package aitools
 
 import (
-	"context"
 	"bufio"
+	"context"
 	"os"
 	"path/filepath"
 	"testing"
diff --git a/experimental/aitools/cmd/skills.go b/experimental/aitools/cmd/skills.go
index 9550eb6b33..8034f818e3 100644
--- a/experimental/aitools/cmd/skills.go
+++ b/experimental/aitools/cmd/skills.go
@@ -2,7 +2,8 @@ package aitools
 
 import (
 	"context"
-	"fmt"
+	"errors"
+
 	"github.com/charmbracelet/huh"
 	"github.com/databricks/cli/experimental/aitools/lib/agents"
 	"github.com/databricks/cli/experimental/aitools/lib/installer"
@@ -13,7 +14,7 @@ import (
 
 // Package-level vars for testability.
 var (
-	promptAgentSelection    = defaultPromptAgentSelection
+	promptAgentSelection     = defaultPromptAgentSelection
 	installSkillsForAgentsFn = installer.InstallSkillsForAgents
 )
 
@@ -37,7 +38,7 @@ func defaultPromptAgentSelection(ctx context.Context, detected []*agents.Agent)
 	}
 
 	if len(selected) == 0 {
-		return nil, fmt.Errorf("at least one agent must be selected")
+		return nil, errors.New("at least one agent must be selected")
 	}
 
 	result := make([]*agents.Agent, 0, len(selected))
diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go
index 7d9ebc95a3..791ba6ffbd 100644
--- a/experimental/aitools/lib/installer/installer_test.go
+++ b/experimental/aitools/lib/installer/installer_test.go
@@ -481,6 +481,6 @@ func TestLegacyDetectLegacyDir(t *testing.T) {
 
 func TestInstallAllSkillsSignaturePreserved(t *testing.T) {
 	// Compile-time check that InstallAllSkills satisfies func(context.Context) error.
-	var fn func(context.Context) error = InstallAllSkills
-	_ = fn
+	callback := func(fn func(context.Context) error) { _ = fn }
+	callback(InstallAllSkills)
 }

From 64c97e157f918ee1e0cbf24219ee818e16112f7b Mon Sep 17 00:00:00 2001
From: simon <simon.faltum@databricks.com>
Date: Mon, 23 Mar 2026 14:32:17 +0100
Subject: [PATCH 6/8] Fix review findings: agent-aware idempotency,
 --experimental flag, legacy guard, local copy

- Make idempotency check agent-aware: verify every requested agent has the
  skill on disk before skipping, so adding a new agent triggers reinstall.
- Add --experimental flag to skills install and top-level install commands
  (previously referenced in error message but missing from CLI).
- Block targeted install (install <skill>) on legacy setups without state
  to prevent writing incomplete state that hides the legacy warning.
- Remove redundant state reload: use single LoadState + in-place mutation.
- Copy files locally from canonical dir instead of re-downloading for
  single-agent and symlink-fallback paths.
- Add IncludeExperimental semantics comment on state field.
- Fix misleading test comment and add NotContains assertion for unchanged skill.

Co-authored-by: Isaac
---
 experimental/aitools/cmd/install.go           |   9 +-
 experimental/aitools/cmd/install_test.go      |  36 ++++--
 experimental/aitools/cmd/skills.go            |  15 ++-
 .../aitools/lib/installer/installer.go        | 119 ++++++++++++------
 .../aitools/lib/installer/installer_test.go   |  95 +++++++++++++-
 5 files changed, 221 insertions(+), 53 deletions(-)

diff --git a/experimental/aitools/cmd/install.go b/experimental/aitools/cmd/install.go
index 7bdc86449c..56f1f034a8 100644
--- a/experimental/aitools/cmd/install.go
+++ b/experimental/aitools/cmd/install.go
@@ -5,14 +5,19 @@ import (
 )
 
 func newInstallCmd() *cobra.Command {
-	return &cobra.Command{
+	var includeExperimental bool
+
+	cmd := &cobra.Command{
 		Use:   "install [skill-name]",
 		Short: "Alias for skills install",
 		Long: `Alias for "databricks experimental aitools skills install".
 
 Installs Databricks skills for detected coding agents.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runSkillsInstall(cmd.Context(), args)
+			return runSkillsInstall(cmd.Context(), args, includeExperimental)
 		},
 	}
+
+	cmd.Flags().BoolVar(&includeExperimental, "experimental", false, "Include experimental skills")
+	return cmd
 }
diff --git a/experimental/aitools/cmd/install_test.go b/experimental/aitools/cmd/install_test.go
index 80a19317b1..bab41a2040 100644
--- a/experimental/aitools/cmd/install_test.go
+++ b/experimental/aitools/cmd/install_test.go
@@ -52,11 +52,13 @@ func TestInstallCommandsDelegateToSkillsInstall(t *testing.T) {
 	calls := setupInstallMock(t)
 
 	tests := []struct {
-		name       string
-		newCmd     func() *cobra.Command
-		args       []string
-		wantAgents int
-		wantSkills []string
+		name               string
+		newCmd             func() *cobra.Command
+		args               []string
+		flags              []string
+		wantAgents         int
+		wantSkills         []string
+		wantExperimental   bool
 	}{
 		{
 			name:       "skills install installs all skills for all agents",
@@ -70,6 +72,13 @@ func TestInstallCommandsDelegateToSkillsInstall(t *testing.T) {
 			wantAgents: 2,
 			wantSkills: []string{"bundle/review"},
 		},
+		{
+			name:             "skills install with --experimental",
+			newCmd:           newSkillsInstallCmd,
+			flags:            []string{"--experimental"},
+			wantAgents:       2,
+			wantExperimental: true,
+		},
 		{
 			name:       "top level install installs all skills",
 			newCmd:     newInstallCmd,
@@ -82,6 +91,13 @@ func TestInstallCommandsDelegateToSkillsInstall(t *testing.T) {
 			wantAgents: 2,
 			wantSkills: []string{"bundle/review"},
 		},
+		{
+			name:             "top level install with --experimental",
+			newCmd:           newInstallCmd,
+			flags:            []string{"--experimental"},
+			wantAgents:       2,
+			wantExperimental: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -91,6 +107,9 @@ func TestInstallCommandsDelegateToSkillsInstall(t *testing.T) {
 			ctx := cmdio.MockDiscard(t.Context())
 			cmd := tt.newCmd()
 			cmd.SetContext(ctx)
+			if len(tt.flags) > 0 {
+				require.NoError(t, cmd.ParseFlags(tt.flags))
+			}
 
 			err := cmd.RunE(cmd, tt.args)
 			require.NoError(t, err)
@@ -98,6 +117,7 @@ func TestInstallCommandsDelegateToSkillsInstall(t *testing.T) {
 			require.Len(t, *calls, 1)
 			assert.Len(t, (*calls)[0].agents, tt.wantAgents)
 			assert.Equal(t, tt.wantSkills, (*calls)[0].opts.SpecificSkills)
+			assert.Equal(t, tt.wantExperimental, (*calls)[0].opts.IncludeExperimental)
 		})
 	}
 }
@@ -133,7 +153,7 @@ func TestRunSkillsInstallInteractivePrompt(t *testing.T) {
 	go drain(test.Stdout)
 	go drain(test.Stderr)
 
-	err := runSkillsInstall(ctx, nil)
+	err := runSkillsInstall(ctx, nil, false)
 	require.NoError(t, err)
 
 	assert.True(t, promptCalled, "prompt should be called when 2+ agents detected and interactive")
@@ -157,7 +177,7 @@ func TestRunSkillsInstallNonInteractiveUsesAllAgents(t *testing.T) {
 	// MockDiscard gives a non-interactive context.
 	ctx := cmdio.MockDiscard(t.Context())
 
-	err := runSkillsInstall(ctx, nil)
+	err := runSkillsInstall(ctx, nil, false)
 	require.NoError(t, err)
 
 	assert.False(t, promptCalled, "prompt should not be called in non-interactive mode")
@@ -173,7 +193,7 @@ func TestRunSkillsInstallNoAgents(t *testing.T) {
 	calls := setupInstallMock(t)
 	ctx := cmdio.MockDiscard(t.Context())
 
-	err := runSkillsInstall(ctx, nil)
+	err := runSkillsInstall(ctx, nil, false)
 	require.NoError(t, err)
 	assert.Empty(t, *calls, "install should not be called when no agents detected")
 }
diff --git a/experimental/aitools/cmd/skills.go b/experimental/aitools/cmd/skills.go
index 8034f818e3..2161cf04ca 100644
--- a/experimental/aitools/cmd/skills.go
+++ b/experimental/aitools/cmd/skills.go
@@ -72,7 +72,9 @@ func newSkillsListCmd() *cobra.Command {
 }
 
 func newSkillsInstallCmd() *cobra.Command {
-	return &cobra.Command{
+	var includeExperimental bool
+
+	cmd := &cobra.Command{
 		Use:   "install [skill-name]",
 		Short: "Install Databricks skills for detected coding agents",
 		Long: `Install Databricks skills to all detected coding agents.
@@ -83,12 +85,15 @@ and symlinked to each agent to avoid duplication.
 
 Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity`,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runSkillsInstall(cmd.Context(), args)
+			return runSkillsInstall(cmd.Context(), args, includeExperimental)
 		},
 	}
+
+	cmd.Flags().BoolVar(&includeExperimental, "experimental", false, "Include experimental skills")
+	return cmd
 }
 
-func runSkillsInstall(ctx context.Context, args []string) error {
+func runSkillsInstall(ctx context.Context, args []string, includeExperimental bool) error {
 	detected := agents.DetectInstalled(ctx)
 	if len(detected) == 0 {
 		cmdio.LogString(ctx, color.YellowString("No supported coding agents detected."))
@@ -115,7 +120,9 @@ func runSkillsInstall(ctx context.Context, args []string) error {
 
 	installer.PrintInstallingFor(ctx, targetAgents)
 
-	opts := installer.InstallOptions{}
+	opts := installer.InstallOptions{
+		IncludeExperimental: includeExperimental,
+	}
 	if len(args) > 0 {
 		opts.SpecificSkills = []string{args[0]}
 	}
diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go
index 2af50222ea..3a1241c2ce 100644
--- a/experimental/aitools/lib/installer/installer.go
+++ b/experimental/aitools/lib/installer/installer.go
@@ -138,8 +138,13 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 	}
 
 	// Detect legacy installs (skills on disk but no state file).
+	// Block targeted installs on legacy setups to avoid writing incomplete state
+	// that would hide the legacy warning on future runs.
 	if state == nil {
-		checkLegacyInstall(ctx, globalDir)
+		isLegacy := checkLegacyInstall(ctx, globalDir)
+		if isLegacy && len(opts.SpecificSkills) > 0 {
+			return fmt.Errorf("legacy install detected without state tracking; run 'databricks experimental aitools skills install' (without a skill name) first to rebuild state")
+		}
 	}
 
 	// Filter skills based on options, experimental flag, and CLI version.
@@ -157,11 +162,12 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 
 	for _, name := range skillNames {
 		meta := targetSkills[name]
-		// Idempotency: skip if same version is already installed and on disk.
+		// Idempotency: skip if same version is already installed, the canonical
+		// dir exists, AND every requested agent already has the skill on disk.
 		if state != nil && state.Skills[name] == meta.Version {
 			skillDir := filepath.Join(globalDir, name)
-			if _, statErr := os.Stat(skillDir); statErr == nil {
-				log.Debugf(ctx, "%s v%s already installed, skipping", name, meta.Version)
+			if _, statErr := os.Stat(skillDir); statErr == nil && allAgentsHaveSkill(ctx, name, targetAgents) {
+				log.Debugf(ctx, "%s v%s already installed for all agents, skipping", name, meta.Version)
 				continue
 			}
 		}
@@ -171,34 +177,24 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 		}
 	}
 
-	// Save state. Merge into existing state so skills from previous installs
-	// (e.g., experimental skills from a prior run) are preserved.
-	existingState, loadErr := LoadState(globalDir)
-	if loadErr != nil {
-		log.Warnf(ctx, "Could not reload state for merge: %v", loadErr)
-	}
-	var newState *InstallState
-	if existingState != nil {
-		newState = existingState
-		newState.Release = latestTag
-		newState.LastUpdated = time.Now()
-		newState.IncludeExperimental = opts.IncludeExperimental
-		for name, meta := range targetSkills {
-			newState.Skills[name] = meta.Version
-		}
-	} else {
-		newState = &InstallState{
-			SchemaVersion:       1,
-			IncludeExperimental: opts.IncludeExperimental,
-			Release:             latestTag,
-			LastUpdated:         time.Now(),
-			Skills:              make(map[string]string, len(targetSkills)),
-		}
-		for name, meta := range targetSkills {
-			newState.Skills[name] = meta.Version
+	// Save state. Merge into existing state (loaded above) so skills from
+	// previous installs (e.g., experimental skills from a prior run) are preserved.
+	if state == nil {
+		state = &InstallState{
+			SchemaVersion: 1,
+			Skills:        make(map[string]string, len(targetSkills)),
 		}
 	}
-	if err := SaveState(globalDir, newState); err != nil {
+	state.Release = latestTag
+	state.LastUpdated = time.Now()
+	// IncludeExperimental reflects the last invocation's flag value. The Skills
+	// map may still contain experimental entries from a prior run with the flag
+	// enabled; this field does not retroactively remove them.
+	state.IncludeExperimental = opts.IncludeExperimental
+	for name, meta := range targetSkills {
+		state.Skills[name] = meta.Version
+	}
+	if err := SaveState(globalDir, state); err != nil {
 		return err
 	}
 
@@ -237,7 +233,7 @@ func resolveSkills(ctx context.Context, skills map[string]SkillMeta, opts Instal
 	for name, meta := range candidates {
 		if meta.Experimental && !opts.IncludeExperimental {
 			if isSpecific {
-				return nil, fmt.Errorf("skill %q is experimental; use --include-experimental to install", name)
+				return nil, fmt.Errorf("skill %q is experimental; use --experimental to install", name)
 			}
 			log.Debugf(ctx, "Skipping experimental skill %s", name)
 			continue
@@ -300,19 +296,22 @@ func printNoAgentsDetected(ctx context.Context) {
 }
 
 // checkLegacyInstall prints a message if skills exist on disk but no state file was found.
-func checkLegacyInstall(ctx context.Context, globalDir string) {
+// Returns true if a legacy install was detected.
+func checkLegacyInstall(ctx context.Context, globalDir string) bool {
 	if hasSkillsOnDisk(globalDir) {
 		cmdio.LogString(ctx, "Found skills installed before state tracking was added. Run 'databricks experimental aitools install' to refresh.")
-		return
+		return true
 	}
 	homeDir, err := env.UserHomeDir(ctx)
 	if err != nil {
-		return
+		return false
 	}
 	legacyDir := filepath.Join(homeDir, ".databricks", "agent-skills")
 	if hasSkillsOnDisk(legacyDir) {
 		cmdio.LogString(ctx, "Found skills installed before state tracking was added. Run 'databricks experimental aitools install' to refresh.")
+		return true
 	}
+	return false
 }
 
 // hasSkillsOnDisk checks if a directory contains subdirectories starting with "databricks".
@@ -329,6 +328,21 @@ func hasSkillsOnDisk(dir string) bool {
 	return false
 }
 
+// allAgentsHaveSkill returns true if every agent in the list has the named
+// skill directory present (either as a real directory or symlink).
+func allAgentsHaveSkill(ctx context.Context, skillName string, targetAgents []*agents.Agent) bool {
+	for _, agent := range targetAgents {
+		agentSkillDir, err := agent.SkillsDir(ctx)
+		if err != nil {
+			return false
+		}
+		if _, err := os.Stat(filepath.Join(agentSkillDir, skillName)); err != nil {
+			return false
+		}
+	}
+	return true
+}
+
 func installSkillForAgents(ctx context.Context, ref, skillName string, files []string, detectedAgents []*agents.Agent, globalDir string) error {
 	canonicalDir := filepath.Join(globalDir, skillName)
 	if err := installSkillToDir(ctx, ref, skillName, canonicalDir, files); err != nil {
@@ -354,14 +368,15 @@ func installSkillForAgents(ctx context.Context, ref, skillName string, files []s
 		if useSymlinks {
 			if err := createSymlink(canonicalDir, destDir); err != nil {
 				log.Debugf(ctx, "Symlink failed for %s, copying instead: %v", agent.DisplayName, err)
-				if err := installSkillToDir(ctx, ref, skillName, destDir, files); err != nil {
+				if err := copyDir(canonicalDir, destDir); err != nil {
 					log.Warnf(ctx, "Failed to install for %s: %v", agent.DisplayName, err)
 					continue
 				}
 			}
 			log.Debugf(ctx, "Installed %q for %s (symlinked)", skillName, agent.DisplayName)
 		} else {
-			if err := installSkillToDir(ctx, ref, skillName, destDir, files); err != nil {
+			// Copy from canonical dir instead of re-downloading.
+			if err := copyDir(canonicalDir, destDir); err != nil {
 				log.Warnf(ctx, "Failed to install for %s: %v", agent.DisplayName, err)
 				continue
 			}
@@ -436,6 +451,38 @@ func installSkillToDir(ctx context.Context, ref, skillName, destDir string, file
 	return nil
 }
 
+// copyDir copies all files from src to dest, recreating the directory structure.
+func copyDir(src, dest string) error {
+	if err := os.RemoveAll(dest); err != nil {
+		return fmt.Errorf("failed to remove existing path: %w", err)
+	}
+
+	return filepath.Walk(src, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		rel, err := filepath.Rel(src, path)
+		if err != nil {
+			return err
+		}
+		target := filepath.Join(dest, rel)
+
+		if info.IsDir() {
+			return os.MkdirAll(target, 0o755)
+		}
+
+		data, err := os.ReadFile(path)
+		if err != nil {
+			return fmt.Errorf("failed to read %s: %w", rel, err)
+		}
+		if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil {
+			return err
+		}
+		return os.WriteFile(target, data, 0o644)
+	})
+}
+
 func createSymlink(source, dest string) error {
 	// ensure parent directory exists
 	if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil {
diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go
index 791ba6ffbd..4a5970002b 100644
--- a/experimental/aitools/lib/installer/installer_test.go
+++ b/experimental/aitools/lib/installer/installer_test.go
@@ -431,10 +431,10 @@ func TestIdempotentInstallUpdatesNewVersions(t *testing.T) {
 	err = InstallSkillsForAgents(ctx, src2, []*agents.Agent{agent}, InstallOptions{})
 	require.NoError(t, err)
 
-	// Both skills should be fetched because the release tag changed.
-	// (databricks-sql has a new version, databricks-jobs has the same version
-	// but state was from v0.1.0 release.)
+	// Only databricks-sql should be re-fetched (version changed from 0.1.0 to 0.2.0).
+	// databricks-jobs keeps version 0.1.0 and should be skipped by the idempotency check.
 	assert.Contains(t, fetchedSkills, "databricks-sql")
+	assert.NotContains(t, fetchedSkills, "databricks-jobs")
 
 	globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
 	state, err := LoadState(globalDir)
@@ -479,6 +479,95 @@ func TestLegacyDetectLegacyDir(t *testing.T) {
 	assert.Contains(t, stderr.String(), "Found skills installed before state tracking was added.")
 }
 
+func TestIdempotentInstallReinstallsForNewAgent(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx := cmdio.MockDiscard(t.Context())
+	setupFetchMock(t)
+
+	src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"}
+	agent1 := testAgent(tmp)
+
+	// First install for agent1.
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent1}, InstallOptions{})
+	require.NoError(t, err)
+
+	// Create a second agent.
+	agent2Dir := filepath.Join(tmp, ".second-agent")
+	require.NoError(t, os.MkdirAll(agent2Dir, 0o755))
+	agent2 := &agents.Agent{
+		Name:        "second-agent",
+		DisplayName: "Second Agent",
+		ConfigDir: func(_ context.Context) (string, error) {
+			return agent2Dir, nil
+		},
+	}
+
+	// Track fetch calls for second install (with both agents).
+	fetchCalls := 0
+	orig := fetchFileFn
+	t.Cleanup(func() { fetchFileFn = orig })
+	fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) {
+		fetchCalls++
+		return []byte("# " + skillName + "/" + filePath), nil
+	}
+
+	// Second install with both agents, same version.
+	err = InstallSkillsForAgents(ctx, src, []*agents.Agent{agent1, agent2}, InstallOptions{})
+	require.NoError(t, err)
+
+	// Skills should be re-fetched because agent2 doesn't have them yet.
+	assert.Greater(t, fetchCalls, 0, "should re-install skills for new agent")
+
+	// Verify agent2 got the skills.
+	agent2SkillsDir := filepath.Join(agent2Dir, "skills")
+	_, err = os.Stat(filepath.Join(agent2SkillsDir, "databricks-sql"))
+	assert.NoError(t, err, "agent2 should have databricks-sql")
+}
+
+func TestLegacyTargetedInstallBlocked(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx := cmdio.MockDiscard(t.Context())
+	setupFetchMock(t)
+
+	// Create skills on disk at canonical location but no state file (legacy).
+	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"}
+	agent := testAgent(tmp)
+
+	// Targeted install should fail on legacy setup.
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{
+		SpecificSkills: []string{"databricks-sql"},
+	})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "legacy install detected")
+}
+
+func TestLegacyFullInstallAllowed(t *testing.T) {
+	tmp := setupTestHome(t)
+	ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
+	setupFetchMock(t)
+
+	// Create skills on disk at canonical location but no state file (legacy).
+	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"}
+	agent := testAgent(tmp)
+
+	// Full install (no SpecificSkills) should succeed and rebuild state.
+	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
+	require.NoError(t, err)
+
+	assert.Contains(t, stderr.String(), "Found skills installed before state tracking was added.")
+
+	state, err := LoadState(globalDir)
+	require.NoError(t, err)
+	require.NotNil(t, state)
+	assert.Len(t, state.Skills, 2)
+}
+
 func TestInstallAllSkillsSignaturePreserved(t *testing.T) {
 	// Compile-time check that InstallAllSkills satisfies func(context.Context) error.
 	callback := func(fn func(context.Context) error) { _ = fn }

From c19ecdc6cbfd7e9050392df94a2008f647671bea Mon Sep 17 00:00:00 2001
From: simon <simon.faltum@databricks.com>
Date: Tue, 24 Mar 2026 12:50:23 +0100
Subject: [PATCH 7/8] Fix lint: gofmt alignment, perfsprint, testifylint

Co-authored-by: Isaac
---
 experimental/aitools/cmd/install_test.go           | 14 +++++++-------
 experimental/aitools/lib/installer/installer.go    |  3 ++-
 .../aitools/lib/installer/installer_test.go        |  2 +-
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/experimental/aitools/cmd/install_test.go b/experimental/aitools/cmd/install_test.go
index bab41a2040..1e1881ffac 100644
--- a/experimental/aitools/cmd/install_test.go
+++ b/experimental/aitools/cmd/install_test.go
@@ -52,13 +52,13 @@ func TestInstallCommandsDelegateToSkillsInstall(t *testing.T) {
 	calls := setupInstallMock(t)
 
 	tests := []struct {
-		name               string
-		newCmd             func() *cobra.Command
-		args               []string
-		flags              []string
-		wantAgents         int
-		wantSkills         []string
-		wantExperimental   bool
+		name             string
+		newCmd           func() *cobra.Command
+		args             []string
+		flags            []string
+		wantAgents       int
+		wantSkills       []string
+		wantExperimental bool
 	}{
 		{
 			name:       "skills install installs all skills for all agents",
diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go
index 3a1241c2ce..15e6a8bbab 100644
--- a/experimental/aitools/lib/installer/installer.go
+++ b/experimental/aitools/lib/installer/installer.go
@@ -2,6 +2,7 @@ package installer
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -143,7 +144,7 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 	if state == nil {
 		isLegacy := checkLegacyInstall(ctx, globalDir)
 		if isLegacy && len(opts.SpecificSkills) > 0 {
-			return fmt.Errorf("legacy install detected without state tracking; run 'databricks experimental aitools skills install' (without a skill name) first to rebuild state")
+			return errors.New("legacy install detected without state tracking; run 'databricks experimental aitools skills install' (without a skill name) first to rebuild state")
 		}
 	}
 
diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go
index 4a5970002b..9764268259 100644
--- a/experimental/aitools/lib/installer/installer_test.go
+++ b/experimental/aitools/lib/installer/installer_test.go
@@ -516,7 +516,7 @@ func TestIdempotentInstallReinstallsForNewAgent(t *testing.T) {
 	require.NoError(t, err)
 
 	// Skills should be re-fetched because agent2 doesn't have them yet.
-	assert.Greater(t, fetchCalls, 0, "should re-install skills for new agent")
+	assert.Positive(t, fetchCalls, "should re-install skills for new agent")
 
 	// Verify agent2 got the skills.
 	agent2SkillsDir := filepath.Join(agent2Dir, "skills")

From 5f87b4a3bf931f075cf1f8c30cc06797e03efc96 Mon Sep 17 00:00:00 2001
From: simon <simon.faltum@databricks.com>
Date: Wed, 25 Mar 2026 23:12:18 +0100
Subject: [PATCH 8/8] Fix rebase: replace FetchLatestRelease with getSkillsRef
 after base branch change

Co-authored-by: Isaac
---
 .../aitools/lib/installer/installer.go        | 14 ++----
 .../aitools/lib/installer/installer_test.go   | 49 +++++++++----------
 2 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go
index 15e6a8bbab..14e9a3cf91 100644
--- a/experimental/aitools/lib/installer/installer.go
+++ b/experimental/aitools/lib/installer/installer.go
@@ -117,12 +117,8 @@ 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)
-	if err != nil {
-		return fmt.Errorf("failed to fetch latest release: %w", err)
-	}
-
-	manifest, err := src.FetchManifest(ctx, latestTag)
+	ref := getSkillsRef(ctx)
+	manifest, err := src.FetchManifest(ctx, ref)
 	if err != nil {
 		return err
 	}
@@ -173,7 +169,7 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 			}
 		}
 
-		if err := installSkillForAgents(ctx, latestTag, name, meta.Files, targetAgents, globalDir); err != nil {
+		if err := installSkillForAgents(ctx, ref, name, meta.Files, targetAgents, globalDir); err != nil {
 			return err
 		}
 	}
@@ -186,7 +182,7 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 			Skills:        make(map[string]string, len(targetSkills)),
 		}
 	}
-	state.Release = latestTag
+	state.Release = ref
 	state.LastUpdated = time.Now()
 	// IncludeExperimental reflects the last invocation's flag value. The Skills
 	// map may still contain experimental entries from a prior run with the flag
@@ -199,7 +195,7 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 		return err
 	}
 
-	tag := strings.TrimPrefix(latestTag, "v")
+	tag := strings.TrimPrefix(ref, "v")
 	noun := "skills"
 	if len(targetSkills) == 1 {
 		noun = "skill"
diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go
index 9764268259..717f5958d4 100644
--- a/experimental/aitools/lib/installer/installer_test.go
+++ b/experimental/aitools/lib/installer/installer_test.go
@@ -19,7 +19,6 @@ import (
 // mockManifestSource is a test double for ManifestSource.
 type mockManifestSource struct {
 	manifest *Manifest
-	release  string
 	fetchErr error
 }
 
@@ -30,10 +29,6 @@ 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 testManifest() *Manifest {
 	return &Manifest{
 		Version:   "1",
@@ -195,7 +190,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()}
 	agent := testAgent(tmp)
 
 	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
@@ -206,12 +201,12 @@ func TestInstallSkillsForAgentsWritesState(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, state)
 	assert.Equal(t, 1, state.SchemaVersion)
-	assert.Equal(t, "v0.1.0", state.Release)
+	assert.Equal(t, defaultSkillsRepoRef, state.Release)
 	assert.Len(t, state.Skills, 2)
 	assert.Equal(t, "0.1.0", state.Skills["databricks-sql"])
 	assert.Equal(t, "0.1.0", state.Skills["databricks-jobs"])
 
-	assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.0).")
+	assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.3).")
 }
 
 func TestInstallSkillForSingleWritesState(t *testing.T) {
@@ -219,7 +214,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()}
 	agent := testAgent(tmp)
 
 	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{
@@ -234,7 +229,7 @@ func TestInstallSkillForSingleWritesState(t *testing.T) {
 	assert.Len(t, state.Skills, 1)
 	assert.Equal(t, "0.1.0", state.Skills["databricks-sql"])
 
-	assert.Contains(t, stderr.String(), "Installed 1 skill (v0.1.0).")
+	assert.Contains(t, stderr.String(), "Installed 1 skill (v0.1.3).")
 }
 
 func TestInstallSkillsSpecificNotFound(t *testing.T) {
@@ -242,7 +237,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()}
 	agent := testAgent(tmp)
 
 	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{
@@ -264,7 +259,7 @@ func TestExperimentalSkillsSkippedByDefault(t *testing.T) {
 		Experimental: true,
 	}
 
-	src := &mockManifestSource{manifest: manifest, release: "v0.1.0"}
+	src := &mockManifestSource{manifest: manifest}
 	agent := testAgent(tmp)
 
 	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
@@ -277,7 +272,7 @@ func TestExperimentalSkillsSkippedByDefault(t *testing.T) {
 	assert.Len(t, state.Skills, 2)
 	assert.NotContains(t, state.Skills, "databricks-experimental")
 
-	assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.0).")
+	assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.3).")
 }
 
 func TestExperimentalSkillsIncludedWithFlag(t *testing.T) {
@@ -292,7 +287,7 @@ func TestExperimentalSkillsIncludedWithFlag(t *testing.T) {
 		Experimental: true,
 	}
 
-	src := &mockManifestSource{manifest: manifest, release: "v0.1.0"}
+	src := &mockManifestSource{manifest: manifest}
 	agent := testAgent(tmp)
 
 	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{
@@ -307,7 +302,7 @@ func TestExperimentalSkillsIncludedWithFlag(t *testing.T) {
 	assert.Contains(t, state.Skills, "databricks-experimental")
 	assert.True(t, state.IncludeExperimental)
 
-	assert.Contains(t, stderr.String(), "Installed 3 skills (v0.1.0).")
+	assert.Contains(t, stderr.String(), "Installed 3 skills (v0.1.3).")
 }
 
 func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) {
@@ -328,7 +323,7 @@ func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) {
 		MinCLIVer: "0.300.0",
 	}
 
-	src := &mockManifestSource{manifest: manifest, release: "v0.1.0"}
+	src := &mockManifestSource{manifest: manifest}
 	agent := testAgent(tmp)
 
 	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
@@ -341,7 +336,7 @@ func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) {
 	assert.Len(t, state.Skills, 2)
 	assert.NotContains(t, state.Skills, "databricks-future")
 
-	assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.0).")
+	assert.Contains(t, stderr.String(), "Installed 2 skills (v0.1.3).")
 	assert.Contains(t, logBuf.String(), "requires CLI version 0.300.0")
 }
 
@@ -358,7 +353,7 @@ func TestMinCLIVersionHardErrorForInstallSingle(t *testing.T) {
 		MinCLIVer: "0.300.0",
 	}
 
-	src := &mockManifestSource{manifest: manifest, release: "v0.1.0"}
+	src := &mockManifestSource{manifest: manifest}
 	agent := testAgent(tmp)
 
 	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{
@@ -374,7 +369,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()}
 	agent := testAgent(tmp)
 
 	// First install.
@@ -403,7 +398,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()}
 	agent := testAgent(tmp)
 
 	// First install.
@@ -416,7 +411,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}
 
 	// Track which skills are fetched.
 	var fetchedSkills []string
@@ -439,7 +434,7 @@ func TestIdempotentInstallUpdatesNewVersions(t *testing.T) {
 	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, defaultSkillsRepoRef, state.Release)
 	assert.Equal(t, "0.2.0", state.Skills["databricks-sql"])
 }
 
@@ -452,7 +447,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()}
 	agent := testAgent(tmp)
 
 	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
@@ -470,7 +465,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()}
 	agent := testAgent(tmp)
 
 	err := InstallSkillsForAgents(ctx, src, []*agents.Agent{agent}, InstallOptions{})
@@ -484,7 +479,7 @@ func TestIdempotentInstallReinstallsForNewAgent(t *testing.T) {
 	ctx := cmdio.MockDiscard(t.Context())
 	setupFetchMock(t)
 
-	src := &mockManifestSource{manifest: testManifest(), release: "v0.1.0"}
+	src := &mockManifestSource{manifest: testManifest()}
 	agent1 := testAgent(tmp)
 
 	// First install for agent1.
@@ -533,7 +528,7 @@ func TestLegacyTargetedInstallBlocked(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()}
 	agent := testAgent(tmp)
 
 	// Targeted install should fail on legacy setup.
@@ -553,7 +548,7 @@ func TestLegacyFullInstallAllowed(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()}
 	agent := testAgent(tmp)
 
 	// Full install (no SpecificSkills) should succeed and rebuild state.