Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,4 @@ Installing Databricks skills for Claude Code...
Using skills version test-ref
Fetching skills manifest...
Warn: --experimental was set but the manifest at test-ref exposes no experimental skills. Set DATABRICKS_SKILLS_REF to a release or ref that includes them.
Downloading test-stable...
Exposing test-stable to 1 agent...
Installed 1 skill.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ Flag --global has been deprecated, use --scope=global
Installing Databricks skills for Claude Code...
Using skills version test-ref
Fetching skills manifest...
Downloading test-stable-a...
Exposing test-stable-a to 1 agent...
Installed 1 skill.

=== install a specific experimental skill
Expand All @@ -17,8 +15,6 @@ Flag --global has been deprecated, use --scope=global
Installing Databricks skills for Claude Code...
Using skills version test-ref
Fetching skills manifest...
Downloading test-exp...
Exposing test-exp to 1 agent...
Installed 1 skill.

=== asking for an experimental skill without --experimental flag errors out
Expand Down
4 changes: 0 additions & 4 deletions acceptance/experimental/aitools/skills/install/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ Flag --global has been deprecated, use --scope=global
Installing Databricks skills for Claude Code...
Using skills version test-ref
Fetching skills manifest...
Downloading test-stable...
Exposing test-stable to 1 agent...
Installed 1 skill.

=== re-run with --experimental installs the experimental one too
Expand All @@ -17,8 +15,6 @@ Flag --global has been deprecated, use --scope=global
Installing Databricks skills for Claude Code...
Using skills version test-ref
Fetching skills manifest...
Downloading test-exp...
Exposing test-exp to 1 agent...
Installed 2 skills.

=== no-op re-run is idempotent (no new fetches, no errors)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,11 @@ Flag --global has been deprecated, use --scope=global
Installing Databricks skills for Cursor...
Using skills version test-ref
Fetching skills manifest...
Downloading alpha...
Exposing alpha to 1 agent...
Downloading beta...
Exposing beta to 1 agent...
Installed 2 skills.

=== update against a release where beta is gone: alpha updates, beta is pruned
>>> DATABRICKS_SKILLS_REF=v2-ref [CLI] experimental aitools update --scope global
Command "update" is deprecated, use "databricks aitools update" instead.
Downloading alpha...
Exposing alpha to 1 agent...
updated alpha v1.0.0 -> v2.0.0
removed beta (no longer in this release)
Updated 1 skill.
Expand Down
15 changes: 14 additions & 1 deletion cmd/aitools/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
// install_test.go.
var (
promptAgentSelection = defaultPromptAgentSelection
promptProceed = defaultPromptProceed
installSkillsForAgentsFn = installer.InstallSkillsForAgents
installPluginForAgentFn = installer.InstallPluginForAgent
recordPluginInstallsFn = installer.RecordPluginInstalls
Expand Down Expand Up @@ -140,7 +141,7 @@ Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Anti
// In the interactive picker path, show a plan summary and confirm.
if !explicit && cmdio.IsPromptSupported(ctx) {
printPlanSummary(ctx, plan, scope)
proceed, err := cmdio.AskYesOrNo(ctx, "Proceed?")
proceed, err := promptProceed(ctx)
if err != nil {
return err
}
Expand Down Expand Up @@ -247,6 +248,18 @@ func agentStateLabel(s agents.DisplayState) string {
}
}

func defaultPromptProceed(_ context.Context) (bool, error) {

@renaudhartert-db renaudhartert-db Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove the context given that it is ignored? It does not seem this function needs to comply with a specific signature.

Suggested change
func defaultPromptProceed(_ context.Context) (bool, error) {
func defaultPromptProceed() (bool, error) {

proceed := true
err := huh.NewConfirm().
Title("Proceed?").
Value(&proceed).
Run()
if err != nil {
return false, err
}
return proceed, nil
}

func defaultPromptAgentSelection(_ context.Context, choices []agentChoice) ([]*agents.Agent, error) {
options := make([]huh.Option[string], 0, len(choices))
byName := make(map[string]*agents.Agent, len(choices))
Expand Down
20 changes: 12 additions & 8 deletions cmd/aitools/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,16 @@ func TestInstallInteractivePickerAndConfirm(t *testing.T) {
return nil, nil
}

// The confirm is a huh widget that reads the real terminal, so drive it via
// the override rather than piped stdin; assert it was consulted.
origProceed := promptProceed
t.Cleanup(func() { promptProceed = origProceed })
proceedCalled := false
promptProceed = func(_ context.Context) (bool, error) {
proceedCalled = true
return true, nil
}

ctx, test := cmdio.SetupTest(t.Context(), cmdio.TestOptions{PromptSupported: true})
defer test.Done()
go drainReader(test.Stdout)
Expand All @@ -329,15 +339,9 @@ func TestInstallInteractivePickerAndConfirm(t *testing.T) {
cmd := NewInstallCmd()
cmd.SetContext(ctx)

errc := make(chan error, 1)
go func() { errc <- cmd.RunE(cmd, nil) }()

_, err := test.Stdin.WriteString("y\n")
require.NoError(t, err)
require.NoError(t, test.Stdin.Flush())

require.NoError(t, <-errc)
require.NoError(t, cmd.RunE(cmd, nil))
assert.True(t, pickerCalled)
assert.True(t, proceedCalled)
require.Len(t, *plugins, 1)
assert.Equal(t, agents.NameClaudeCode, (*plugins)[0].agent)
}
Expand Down
55 changes: 19 additions & 36 deletions libs/aitools/installer/installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,14 +316,15 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
return fmt.Errorf("failed to load install state: %w", err)
}

// Detect legacy installs (skills on disk but no state file). Global only.
// Block targeted installs on legacy setups to avoid writing incomplete state
// that would hide the legacy warning on future runs.
if state == nil && scope == ScopeGlobal {
isLegacy := checkLegacyInstall(ctx, baseDir)
if isLegacy && len(opts.SpecificSkills) > 0 {
return errors.New("legacy install detected without state tracking; run 'databricks aitools install' (without a skill name) first to rebuild state")
}
// A "legacy install" is one from before the CLI tracked install state in a
// JSON file: skills exist on disk but there's no state file (state == nil).
// A targeted install (--skills foo) here would write a state file listing only
// foo, leaving the other on-disk skills untracked. Block it so the user runs a
// full install first, which rebuilds complete state. Global scope only, since
// legacy installs only ever wrote to the global dir.
// hasLegacyInstall lives in update.go (shared with the update/uninstall paths).
if state == nil && scope == ScopeGlobal && len(opts.SpecificSkills) > 0 && hasLegacyInstall(ctx, baseDir) {
return errors.New("legacy install detected without state tracking; run 'databricks aitools install' (without a skill name) first to rebuild state")
}

// Filter skills based on options, experimental flag, and CLI version.
Expand All @@ -342,6 +343,11 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
// Install each skill in sorted order for determinism.
skillNames := slices.Sorted(maps.Keys(targetSkills))

// Scoped to the loop, not the manifest fetch above, so earlier warnings print
// as plain lines instead of racing the live spinner's redraw.
sp := cmdio.NewSpinner(ctx)
defer sp.Close()

// Accumulate file provenance for skills we (re)fetch this run. Skipped
// (already-installed) skills keep their existing records via the merge below.
fileRecords := map[string]FileRecord{}
Expand All @@ -360,6 +366,7 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
}
}

sp.Update("Installing " + name + "...")
records, err := installSkillForAgents(ctx, name, meta, targetAgents, params)
if err != nil {
return err
Expand Down Expand Up @@ -407,6 +414,10 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
return err
}

// Stop the spinner before the summary so its transient line doesn't linger
// above the persistent "Installed N skills." output.
sp.Close()

noun := "skills"
if len(targetSkills) == 1 {
noun = "skill"
Expand Down Expand Up @@ -522,25 +533,6 @@ func printNoAgentsDetected(ctx context.Context) {
cmdio.LogString(ctx, "Please install at least one coding agent first.")
}

// checkLegacyInstall prints a message if skills exist on disk but no state file was found.
// Returns true if a legacy install was detected.
func checkLegacyInstall(ctx context.Context, globalDir string) bool {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYA, this function was removed in favor of reusing the one at

func hasLegacyInstall(ctx context.Context, globalDir string) bool {
. The error message here was deemed a 'scary legacy warning' by Claude.

if hasSkillsOnDisk(globalDir) {
cmdio.LogString(ctx, "Found skills installed before state tracking was added. Run 'databricks aitools install' to refresh.")
return true
}
homeDir, err := env.UserHomeDir(ctx)
if err != nil {
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 aitools install' to refresh.")
return true
}
return false
}

// hasSkillsOnDisk checks if a directory contains subdirectories starting with "databricks".
func hasSkillsOnDisk(dir string) bool {
entries, err := os.ReadDir(dir)
Expand Down Expand Up @@ -580,15 +572,13 @@ type installParams struct {

func installSkillForAgents(ctx context.Context, skillName string, meta SkillMeta, detectedAgents []*agents.Agent, params installParams) (map[string]FileRecord, error) {
canonicalDir := filepath.Join(params.baseDir, skillName)
cmdio.LogString(ctx, fmt.Sprintf("Downloading %s...", skillName))
records, err := installSkillToDir(ctx, params.ref, meta.RepoDir, meta.SourceName, canonicalDir, meta.Files)
if err != nil {
return nil, err
}

// For project scope, always symlink. For global, symlink when multiple agents.
useSymlinks := params.scope == ScopeProject || len(detectedAgents) > 1
cmdio.LogString(ctx, fmt.Sprintf("Exposing %s to %d %s...", skillName, len(detectedAgents), agentNoun(len(detectedAgents))))

for _, agent := range detectedAgents {
agentSkillDir, err := agentSkillsDirForScope(ctx, agent, params.scope, params.cwd)
Expand Down Expand Up @@ -634,13 +624,6 @@ func installSkillForAgents(ctx context.Context, skillName string, meta SkillMeta
return records, nil
}

func agentNoun(n int) string {
if n == 1 {
return "agent"
}
return "agents"
}

// agentSkillsDirForScope returns the agent's skills directory for the given scope.
func agentSkillsDirForScope(ctx context.Context, agent *agents.Agent, scope, cwd string) (string, error) {
if scope == ScopeProject {
Expand Down
45 changes: 15 additions & 30 deletions libs/aitools/installer/installer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,9 +439,10 @@ func TestInstallSkillsForAgentsWritesState(t *testing.T) {
assert.NotEmpty(t, state.Files["databricks-sql/SKILL.md"].SHA256)
assert.Equal(t, testSkillsRef, state.Files["databricks-sql/SKILL.md"].Origin)

// Per-skill progress now goes through a spinner, which is silent in this
// non-interactive test, so only these plain lines remain.
assert.Contains(t, stderr.String(), "Using skills version")
assert.Contains(t, stderr.String(), "Fetching skills manifest...")
assert.Contains(t, stderr.String(), "Downloading databricks-sql...")
assert.Contains(t, stderr.String(), "Exposing databricks-sql to 1 agent...")
assert.Contains(t, stderr.String(), "Installed 2 skills.")
}

Expand Down Expand Up @@ -713,42 +714,26 @@ func TestIdempotentInstallUpdatesNewVersions(t *testing.T) {
assert.Equal(t, "0.2.0", state.Skills["databricks-sql"])
}

func TestLegacyDetectMessagePrinted(t *testing.T) {
func TestLegacyTargetedInstallBlockedFromLegacyDir(t *testing.T) {
tmp := setupTestHome(t)
ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
setupFetchMock(t)
t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef)

// 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()}
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())
ctx := cmdio.MockDiscard(t.Context())
setupFetchMock(t)
t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef)

// Create skills in the legacy location.
// Skills in the legacy location (~/.databricks/agent-skills), no state file.
legacyDir := filepath.Join(tmp, ".databricks", "agent-skills")
require.NoError(t, os.MkdirAll(filepath.Join(legacyDir, "databricks-sql"), 0o755))

src := &mockManifestSource{manifest: testManifest()}
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.")
// A targeted install must be blocked when a legacy install is detected via
// the legacy dir, mirroring the canonical-dir case.
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 TestIdempotentInstallReinstallsForNewAgent(t *testing.T) {
Expand Down Expand Up @@ -831,11 +816,11 @@ func TestLegacyFullInstallAllowed(t *testing.T) {
src := &mockManifestSource{manifest: testManifest()}
agent := testAgent(tmp)

// Full install (no SpecificSkills) should succeed and rebuild state.
// Full install (no SpecificSkills) should succeed and rebuild state silently.
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.")
assert.Contains(t, stderr.String(), "Installed 2 skills.")

state, err := LoadState(globalDir)
require.NoError(t, err)
Expand Down
5 changes: 5 additions & 0 deletions libs/aitools/installer/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,15 +187,20 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent
ref: latestTag,
}

sp := cmdio.NewSpinner(ctx)
defer sp.Close()

fileRecords := map[string]FileRecord{}
for _, change := range allChanges {
meta := manifest.Skills[change.Name]
sp.Update("Installing " + change.Name + "...")
records, err := installSkillForAgents(ctx, change.Name, meta, targetAgents, params)
if err != nil {
return nil, err
}
maps.Copy(fileRecords, records)
}
sp.Close()

// Update state.
state.Release = latestTag
Expand Down
Loading