diff --git a/acceptance/experimental/aitools/skills/update-prune/output.txt b/acceptance/experimental/aitools/skills/update-prune/output.txt index b6d9c3ed25..5cc1a8c5bd 100644 --- a/acceptance/experimental/aitools/skills/update-prune/output.txt +++ b/acceptance/experimental/aitools/skills/update-prune/output.txt @@ -15,6 +15,8 @@ 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. +Installing databricks plugin for Claude Code... +Skipped Claude Code: claude-code: install-failed: ✘ Failed to install plugin "databricks@claude-plugins-official": Plugin "databricks" not found in marketplace "claude-plugins-official". Your local copy may be out of date — try `claude plugin marketplace update claude-plugins-official`. Downloading alpha... Exposing alpha to 1 agent... updated alpha v1.0.0 -> v2.0.0 diff --git a/cmd/aitools/install_test.go b/cmd/aitools/install_test.go index 415821829e..bd3394eae3 100644 --- a/cmd/aitools/install_test.go +++ b/cmd/aitools/install_test.go @@ -147,15 +147,15 @@ func TestAgentChoicesOnlyOffersActionableAgents(t *testing.T) { fakeBinsOnPath(t, "claude") ctx := cmdio.MockDiscard(t.Context()) - // Project scope: only Claude (plugin) and Cursor (skills) support it; the - // user-only plugin agents and files-only agents are not offered as choices. + // Project scope: only Claude (plugin) supports it; the user-only plugin + // agents and files-only agents are not offered as choices. choices := agentChoices(ctx, installer.ScopeProject, false) var names []string for _, c := range choices { names = append(names, c.agent.Name) } assert.Contains(t, names, agents.NameClaudeCode) - assert.Contains(t, names, agents.NameCursor) + assert.NotContains(t, names, agents.NameCursor) assert.NotContains(t, names, agents.NameCodex) assert.NotContains(t, names, agents.NameOpenCode) assert.NotContains(t, names, agents.NameCopilot) diff --git a/cmd/aitools/list.go b/cmd/aitools/list.go index 75bff4ecf0..b958413787 100644 --- a/cmd/aitools/list.go +++ b/cmd/aitools/list.go @@ -89,7 +89,8 @@ type agentEntry struct { // pluginInfo is the per-scope plugin record surfaced in list output. type pluginInfo struct { - Version string `json:"version,omitempty"` + Version string `json:"version,omitempty"` + NativeScope string `json:"native_scope,omitempty"` } type skillEntry struct { @@ -213,7 +214,7 @@ func buildAgentEntries(states map[string]*installer.InstallState) []agentEntry { installed := map[string]pluginInfo{} for scope, st := range states { if rec, ok := st.Plugins[a.Name]; ok { - installed[scope] = pluginInfo{Version: rec.Version} + installed[scope] = pluginInfo{Version: rec.Version, NativeScope: rec.Scope} } } if len(installed) > 0 { @@ -265,29 +266,31 @@ func renderListText(ctx context.Context, out listOutput, scope string) { } } - cmdio.LogString(ctx, "Available skills ("+versionToken(out.Release)+"):") - cmdio.LogString(ctx, "") - cmdio.LogString(ctx, renderSkillTable(stable, bothScopes)) - - if len(experimental) > 0 { - cmdio.LogString(ctx, "Experimental skills:") - cmdio.LogString(ctx, "") - cmdio.LogString(ctx, renderSkillTable(experimental, bothScopes)) - } - - cmdio.LogString(ctx, summaryLine(out, scope)) - if len(out.Agents) > 0 { + cmdio.LogString(ctx, "Plugin installs:") cmdio.LogString(ctx, "") var ab strings.Builder atw := tabwriter.NewWriter(&ab, 0, 4, 2, ' ', 0) fmt.Fprintln(atw, " AGENT\tSTATUS") for _, a := range out.Agents { - fmt.Fprintf(atw, " %s\t%s\n", a.Name, agentStatusLabel(a, out.Release)) + fmt.Fprintf(atw, " %s\t%s\n", agentDisplayName(a.Name), agentStatusLabel(a, out.Release)) } atw.Flush() cmdio.LogString(ctx, ab.String()) + cmdio.LogString(ctx, "") } + + cmdio.LogString(ctx, "Available raw skill directories ("+versionToken(out.Release)+"):") + cmdio.LogString(ctx, "") + cmdio.LogString(ctx, renderSkillTable(stable, bothScopes)) + + if len(experimental) > 0 { + cmdio.LogString(ctx, "Experimental skills:") + cmdio.LogString(ctx, "") + cmdio.LogString(ctx, renderSkillTable(experimental, bothScopes)) + } + + cmdio.LogString(ctx, summaryLine(out, scope)) } // renderSkillTable formats a NAME/VERSION/INSTALLED table for a group of skills. @@ -323,9 +326,9 @@ func agentStatusLabel(a agentEntry, release string) string { } if upToDate { - return "plugin · " + versionToken(version) + " · up to date" + return "databricks plugin · " + versionToken(version) + " · up to date" } - return "plugin · " + versionToken(version) + " · update available" + return "databricks plugin · " + versionToken(version) + " · update available" } func installedStatusFromEntry(s skillEntry, bothScopes bool) string { @@ -372,15 +375,15 @@ func summaryLine(out listOutput, scope string) string { // Mirror prior behavior: only print the dual-scope line when both // scopes have a state file; otherwise only mention the one that does. if g.loaded && p.loaded { - return fmt.Sprintf("%d/%d skills installed (global), %d/%d (project)", g.Installed, g.Total, p.Installed, p.Total) + return fmt.Sprintf("%d/%d raw skill directories installed (global), %d/%d (project)", g.Installed, g.Total, p.Installed, p.Total) } if p.loaded { - return fmt.Sprintf("%d/%d skills installed (project)", p.Installed, p.Total) + return fmt.Sprintf("%d/%d raw skill directories installed (project)", p.Installed, p.Total) } - return fmt.Sprintf("%d/%d skills installed (global)", g.Installed, g.Total) + return fmt.Sprintf("%d/%d raw skill directories installed (global)", g.Installed, g.Total) case pOK: - return fmt.Sprintf("%d/%d skills installed (project)", p.Installed, p.Total) + return fmt.Sprintf("%d/%d raw skill directories installed (project)", p.Installed, p.Total) default: - return fmt.Sprintf("%d/%d skills installed (global)", g.Installed, g.Total) + return fmt.Sprintf("%d/%d raw skill directories installed (global)", g.Installed, g.Total) } } diff --git a/cmd/aitools/list_test.go b/cmd/aitools/list_test.go index 40cc7a271b..c977ac1b5c 100644 --- a/cmd/aitools/list_test.go +++ b/cmd/aitools/list_test.go @@ -156,12 +156,12 @@ func TestBuildAgentEntries(t *testing.T) { require.Contains(t, byName, "claude-code") assert.True(t, byName["claude-code"].Managed) assert.Equal(t, "0.2.6", byName["claude-code"].Installed[installer.ScopeGlobal].Version) - assert.Equal(t, "plugin · v0.2.6 · up to date", agentStatusLabel(byName["claude-code"], "0.2.6")) + assert.Equal(t, "databricks plugin · v0.2.6 · up to date", agentStatusLabel(byName["claude-code"], "0.2.6")) require.Contains(t, byName, "codex") assert.True(t, byName["codex"].Managed) assert.Equal(t, "0.2.5", byName["codex"].Installed[installer.ScopeGlobal].Version) - assert.Equal(t, "plugin · v0.2.5 · update available", agentStatusLabel(byName["codex"], "0.2.6")) + assert.Equal(t, "databricks plugin · v0.2.5 · update available", agentStatusLabel(byName["codex"], "0.2.6")) // Cursor has no plugin, so it never appears as a plugin agent entry. assert.NotContains(t, byName, "cursor") @@ -194,7 +194,7 @@ func TestBuildAgentEntriesRecordsPerScopeVersions(t *testing.T) { // The renderer collapses the scopes and surfaces the stale one, rather than // hiding it behind the up-to-date scope. - assert.Equal(t, "plugin · v0.2.5 · update available", agentStatusLabel(cc, "0.2.6")) + assert.Equal(t, "databricks plugin · v0.2.5 · update available", agentStatusLabel(cc, "0.2.6")) } func TestRenderListJSONScopeFiltersSummary(t *testing.T) { @@ -282,7 +282,7 @@ func TestSummaryLinePreservesStatePresence(t *testing.T) { installer.ScopeProject: {Installed: 0, Total: 1, loaded: true}, }, }, - want: "0/1 skills installed (global), 0/1 (project)", + want: "0/1 raw skill directories installed (global), 0/1 (project)", }, { name: "only project state loaded", @@ -295,7 +295,7 @@ func TestSummaryLinePreservesStatePresence(t *testing.T) { installer.ScopeProject: {Installed: 0, Total: 1, loaded: true}, }, }, - want: "0/1 skills installed (project)", + want: "0/1 raw skill directories installed (project)", }, { name: "only global state loaded", @@ -308,7 +308,7 @@ func TestSummaryLinePreservesStatePresence(t *testing.T) { installer.ScopeProject: {Installed: 0, Total: 1}, }, }, - want: "0/1 skills installed (global)", + want: "0/1 raw skill directories installed (global)", }, } @@ -342,7 +342,7 @@ func TestRenderListTextUsesLoadedStateForScopeLabels(t *testing.T) { got := stderr.String() assert.Contains(t, got, "v1.0.0 (up to date) (global)") - assert.Contains(t, got, "1/1 skills installed (global), 0/1 (project)") + assert.Contains(t, got, "1/1 raw skill directories installed (global), 0/1 (project)") } func TestRenderListTextGroupsExperimental(t *testing.T) { @@ -361,7 +361,7 @@ func TestRenderListTextGroupsExperimental(t *testing.T) { renderListText(ctx, out, installer.ScopeGlobal) got := stderr.String() - availIdx := strings.Index(got, "Available skills") + availIdx := strings.Index(got, "Available raw skill directories") expIdx := strings.Index(got, "Experimental skills:") require.GreaterOrEqual(t, availIdx, 0, "available group header present") require.GreaterOrEqual(t, expIdx, 0, "experimental group header present") @@ -373,6 +373,38 @@ func TestRenderListTextGroupsExperimental(t *testing.T) { assert.NotContains(t, got, "[experimental]") } +func TestRenderListTextShowsPluginInstallsBeforeRawSkills(t *testing.T) { + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + out := listOutput{ + Release: "0.2.6", + Skills: []skillEntry{ + {Name: "databricks-jobs", LatestVersion: "1.0.0", Installed: map[string]string{}}, + }, + Summary: map[string]scopeSummary{ + installer.ScopeGlobal: {Installed: 0, Total: 1, loaded: true}, + }, + Agents: []agentEntry{ + { + Name: "claude-code", + Managed: true, + Installed: map[string]pluginInfo{installer.ScopeGlobal: {Version: "0.2.6", NativeScope: "user"}}, + }, + }, + } + + renderListText(ctx, out, installer.ScopeGlobal) + + got := stderr.String() + pluginIdx := strings.Index(got, "Plugin installs:") + rawIdx := strings.Index(got, "Available raw skill directories") + require.GreaterOrEqual(t, pluginIdx, 0) + require.GreaterOrEqual(t, rawIdx, 0) + assert.Less(t, pluginIdx, rawIdx) + assert.Contains(t, got, "Claude Code") + assert.Contains(t, got, "databricks plugin · v0.2.6 · up to date") + assert.Contains(t, got, "0/1 raw skill directories installed (global)") +} + func TestListScopeFlag(t *testing.T) { tests := []struct { name string diff --git a/cmd/aitools/uninstall.go b/cmd/aitools/uninstall.go index 2bf53f9ad9..3336f92e6d 100644 --- a/cmd/aitools/uninstall.go +++ b/cmd/aitools/uninstall.go @@ -16,7 +16,7 @@ var uninstallSkillsFn = func(ctx context.Context, opts installer.UninstallOption } func NewUninstallCmd() *cobra.Command { - var skillsFlag, scopeFlag string + var skillsFlag, agentsFlag, scopeFlag string var projectFlag, globalFlag, keepMarketplace bool cmd := &cobra.Command{ @@ -53,6 +53,16 @@ By default, removes all skills. Use --skills to remove specific skills only.`, KeepMarketplace: keepMarketplace, } opts.Skills = splitAndTrim(skillsFlag) + if agentsFlag != "" { + targetAgents, err := resolveAgentNames(ctx, agentsFlag) + if err != nil { + return err + } + opts.Agents = make([]string, 0, len(targetAgents)) + for _, agent := range targetAgents { + opts.Agents = append(opts.Agents, agent.Name) + } + } // Uninstall is destructive, so confirm interactively before doing // anything. Non-interactive runs (no TTY) proceed unprompted so @@ -77,6 +87,7 @@ By default, removes all skills. Use --skills to remove specific skills only.`, } cmd.Flags().StringVar(&skillsFlag, "skills", "", "Specific skills to uninstall (comma-separated)") + cmd.Flags().StringVar(&agentsFlag, "agents", "", "Agents to uninstall from (comma-separated, e.g. claude-code,cursor)") cmd.Flags().BoolVar(&keepMarketplace, "keep-marketplace", false, "Keep the marketplace registration when removing a plugin") cmd.Flags().StringVar(&scopeFlag, "scope", "", "Uninstall scope: project or global") cmd.Flags().BoolVar(&projectFlag, "project", false, "Uninstall project-scoped skills") @@ -108,8 +119,12 @@ func uninstallConfirmMessage(state *installer.InstallState, opts installer.Unins if state == nil { return "", false } + target := "all agents" + if len(opts.Agents) > 0 { + target = strings.Join(opts.Agents, ", ") + } if len(opts.Skills) > 0 { - return fmt.Sprintf("This will remove %s %s (%s scope).", plural(len(opts.Skills), "skill"), strings.Join(opts.Skills, ", "), opts.Scope), true + return fmt.Sprintf("This will remove %s %s from %s (%s scope).", plural(len(opts.Skills), "skill"), strings.Join(opts.Skills, ", "), target, opts.Scope), true } var parts []string if n := len(state.Skills); n > 0 { @@ -121,7 +136,7 @@ func uninstallConfirmMessage(state *installer.InstallState, opts installer.Unins if len(parts) == 0 { return "", false } - return fmt.Sprintf("This will remove %s (%s scope).", strings.Join(parts, " and "), opts.Scope), true + return fmt.Sprintf("This will remove %s from %s (%s scope).", strings.Join(parts, " and "), target, opts.Scope), true } // plural returns noun for n == 1 and noun+"s" otherwise. diff --git a/cmd/aitools/uninstall_test.go b/cmd/aitools/uninstall_test.go index f9a8016a82..04c70393c3 100644 --- a/cmd/aitools/uninstall_test.go +++ b/cmd/aitools/uninstall_test.go @@ -65,6 +65,37 @@ func TestUninstallScopeFlag(t *testing.T) { } } +func TestUninstallAgentsFlag(t *testing.T) { + setupTestAgents(t) + calls := setupUninstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := NewUninstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--scope", "global", "--agents", "cursor,claude-code", "--skills", "databricks-sql"}) + + require.NoError(t, cmd.Execute()) + require.Len(t, *calls, 1) + assert.Equal(t, []string{"cursor", "claude-code"}, (*calls)[0].Agents) + assert.Equal(t, []string{"databricks-sql"}, (*calls)[0].Skills) +} + +func TestUninstallUnknownAgentErrors(t *testing.T) { + setupTestAgents(t) + setupUninstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := NewUninstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--scope", "global", "--agents", "invalid-agent"}) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "unknown agent") +} + func TestUninstallConfirmMessage(t *testing.T) { // Nothing recorded: no prompt (installer surfaces its own guidance). _, ask := uninstallConfirmMessage(nil, installer.UninstallOptions{Scope: installer.ScopeGlobal}) @@ -90,5 +121,12 @@ func TestUninstallConfirmMessage(t *testing.T) { msg2, ask2 := uninstallConfirmMessage(st, filtered) require.True(t, ask2) assert.Contains(t, msg2, "skill alpha") + assert.Contains(t, msg2, "from all agents") assert.Contains(t, msg2, "(project scope)") + + targeted := installer.UninstallOptions{Scope: installer.ScopeGlobal, Agents: []string{"cursor"}} + targeted.Skills = []string{"alpha"} + msg3, ask3 := uninstallConfirmMessage(st, targeted) + require.True(t, ask3) + assert.Contains(t, msg3, "from cursor") } diff --git a/cmd/aitools/update.go b/cmd/aitools/update.go index a0d3be2896..0a521e9743 100644 --- a/cmd/aitools/update.go +++ b/cmd/aitools/update.go @@ -3,10 +3,13 @@ package aitools import ( "context" "fmt" + "maps" + "slices" "github.com/databricks/cli/libs/aitools/agents" "github.com/databricks/cli/libs/aitools/installer" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/log" "github.com/spf13/cobra" ) @@ -15,7 +18,23 @@ var updateSkillsFn = func(ctx context.Context, src installer.ManifestSource, ins return installer.UpdateSkills(ctx, src, installed, opts) } -var updatePluginsFn = installer.UpdateInstalledPlugins +var ( + updatePluginsFn = installer.UpdateInstalledPlugins + updateInstallPluginForAgentFn = installer.InstallPluginForAgent + updateRecordPluginInstallsFn = installer.RecordPluginInstalls + updateCleanupLegacyFn = installer.RemoveLegacyRawSkills + hasManagedRawSkillsForAgentFn = installer.HasManagedRawSkillsForAgent +) + +type pluginMigrationCandidate struct { + agent *agents.Agent + nativeScope string +} + +type pluginMigrationResult struct { + agent *agents.Agent + record installer.PluginRecord +} func NewUpdateCmd() *cobra.Command { var check, force, noNew, noPrune bool @@ -75,6 +94,18 @@ preview what would change without downloading.`, return err } + if check && refErr == nil && state != nil && len(state.Plugins) > 0 { + printPluginCheckResults(ctx, state, installer.DisplaySkillsVersion(ref)) + } + + migrationCandidates, err := pluginMigrationCandidatesForScope(ctx, scope, state) + if err != nil { + return err + } + if check { + printPluginMigrationCheckResults(ctx, migrationCandidates) + } + // Update plugin agents through their own CLI (check mode skips this). if !check && refErr == nil { pluginUpdates, err := updatePluginsFn(ctx, scope, ref) @@ -84,6 +115,17 @@ preview what would change without downloading.`, for _, pu := range pluginUpdates { cmdio.LogString(ctx, fmt.Sprintf(" %s databricks plugin %s", pu.Agent, versionToken(pu.Version))) } + + migrated, err := migrateLegacyRawSkillsToPlugins(ctx, scope, ref, migrationCandidates) + if err != nil { + return err + } + if migrated { + state, err = installer.LoadState(dir) + if err != nil { + return err + } + } } // Reconcile file skills for non-plugin agents. Skip entirely for a @@ -103,8 +145,11 @@ preview what would change without downloading.`, if err != nil { return err } - if result != nil && (len(result.Updated) > 0 || len(result.Added) > 0 || len(result.Removed) > 0) { - cmdio.LogString(ctx, installer.FormatUpdateResult(result, check)) + if result != nil && (check || len(result.Updated) > 0 || len(result.Added) > 0 || len(result.Removed) > 0) { + text := installer.FormatUpdateResult(result, check) + if text != "No changes." || len(migrationCandidates) == 0 { + cmdio.LogString(ctx, text) + } } } } @@ -124,6 +169,88 @@ preview what would change without downloading.`, return cmd } +func pluginMigrationCandidatesForScope(ctx context.Context, scope string, state *installer.InstallState) ([]pluginMigrationCandidate, error) { + var candidates []pluginMigrationCandidate + for _, agent := range agents.Registry { + if agent.Plugin == nil { + continue + } + if state != nil { + if _, alreadyPlugin := state.Plugins[agent.Name]; alreadyPlugin { + continue + } + } + nativeScope, ok, _ := mapAgentScope(agent, scope) + if !ok { + continue + } + // Match install's non-interactive behavior: a raw install for a plugin + // agent should attempt migration and report a skip if its CLI is missing, + // rather than silently staying on raw skills forever. + hasRawSkills, err := hasManagedRawSkillsForAgentFn(ctx, agent, scope) + if err != nil { + return nil, err + } + if !hasRawSkills { + continue + } + candidates = append(candidates, pluginMigrationCandidate{agent: agent, nativeScope: nativeScope}) + } + return candidates, nil +} + +func printPluginMigrationCheckResults(ctx context.Context, candidates []pluginMigrationCandidate) { + for _, candidate := range candidates { + cmdio.LogString(ctx, fmt.Sprintf(" %s would migrate raw skills to databricks plugin", candidate.agent.DisplayName)) + } +} + +func migrateLegacyRawSkillsToPlugins(ctx context.Context, scope, ref string, candidates []pluginMigrationCandidate) (bool, error) { + if len(candidates) == 0 { + return false, nil + } + + records := map[string]installer.PluginRecord{} + migrated := make([]pluginMigrationResult, 0, len(candidates)) + for _, candidate := range candidates { + agent := candidate.agent + cmdio.LogString(ctx, fmt.Sprintf("Installing databricks plugin for %s...", agent.DisplayName)) + rec, err := updateInstallPluginForAgentFn(ctx, agent, candidate.nativeScope, ref) + if err != nil { + cmdio.LogString(ctx, cmdio.Yellow(ctx, fmt.Sprintf("Skipped %s: %v", agent.DisplayName, err))) + continue + } + records[agent.Name] = rec + migrated = append(migrated, pluginMigrationResult{agent: agent, record: rec}) + } + if len(records) == 0 { + return false, nil + } + if err := updateRecordPluginInstallsFn(ctx, scope, records, ref); err != nil { + return false, err + } + for _, result := range migrated { + if err := updateCleanupLegacyFn(ctx, result.agent, scope); err != nil { + log.Debugf(ctx, "Legacy skill cleanup for %s failed: %v", result.agent.DisplayName, err) + } + cmdio.LogString(ctx, fmt.Sprintf(" %s databricks plugin %s", result.agent.DisplayName, versionToken(result.record.Version))) + } + return true, nil +} + +func printPluginCheckResults(ctx context.Context, state *installer.InstallState, latest string) { + for _, name := range slices.Sorted(maps.Keys(state.Plugins)) { + rec := state.Plugins[name] + status := "up to date" + if rec.Version == "" { + status = "version unknown" + } else if rec.Version != latest { + status = "update available" + } + cmdio.LogString(ctx, fmt.Sprintf(" %s databricks plugin %s (%s)", agentDisplayName(name), versionToken(rec.Version), status)) + } +} + // excludePluginAgents drops agents that are managed as plugins in this scope, so // the file-skills reconcile never drops duplicate skill files onto a plugin agent. func excludePluginAgents(installed []*agents.Agent, state *installer.InstallState) []*agents.Agent { diff --git a/cmd/aitools/update_test.go b/cmd/aitools/update_test.go index 5016fd9a18..2dee55bc5c 100644 --- a/cmd/aitools/update_test.go +++ b/cmd/aitools/update_test.go @@ -2,6 +2,7 @@ package aitools import ( "context" + "errors" "testing" "github.com/databricks/cli/libs/aitools/agents" @@ -24,6 +25,308 @@ func setupUpdateMock(t *testing.T) *[]installer.UpdateOptions { return &calls } +func resetUpdatePluginSeams(t *testing.T) { + t.Helper() + origUpdatePlugins := updatePluginsFn + origInstallPlugin := updateInstallPluginForAgentFn + origRecordPlugins := updateRecordPluginInstallsFn + origCleanupLegacy := updateCleanupLegacyFn + origHasManagedRawSkills := hasManagedRawSkillsForAgentFn + t.Cleanup(func() { + updatePluginsFn = origUpdatePlugins + updateInstallPluginForAgentFn = origInstallPlugin + updateRecordPluginInstallsFn = origRecordPlugins + updateCleanupLegacyFn = origCleanupLegacy + hasManagedRawSkillsForAgentFn = origHasManagedRawSkills + }) +} + +func TestUpdateCheckPrintsNoChanges(t *testing.T) { + setupTestAgents(t) + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.6") + setupUpdateMock(t) + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + cmd := NewUpdateCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--scope", "global", "--check"}) + + require.NoError(t, cmd.Execute()) + assert.Contains(t, stderr.String(), "No changes.") +} + +func TestUpdateCheckPrintsPluginState(t *testing.T) { + setupTestAgents(t) + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.6") + + origUpdateSkills := updateSkillsFn + origUpdatePlugins := updatePluginsFn + t.Cleanup(func() { + updateSkillsFn = origUpdateSkills + updatePluginsFn = origUpdatePlugins + }) + updateSkillsFn = func(context.Context, installer.ManifestSource, []*agents.Agent, installer.UpdateOptions) (*installer.UpdateResult, error) { + require.Fail(t, "pure plugin check should not update raw skills") + return nil, nil + } + updatePluginsFn = func(context.Context, string, string) ([]installer.PluginUpdate, error) { + require.Fail(t, "check mode must not run plugin update commands") + return nil, nil + } + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + dir, err := installer.GlobalSkillsDir(ctx) + require.NoError(t, err) + require.NoError(t, installer.SaveState(dir, &installer.InstallState{ + SchemaVersion: 2, + Release: "v0.2.6", + Plugins: map[string]installer.PluginRecord{ + agents.NameClaudeCode: {Marketplace: "claude-plugins-official", Plugin: "databricks", Version: "0.2.6"}, + }, + })) + + cmd := NewUpdateCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--scope", "global", "--check"}) + + require.NoError(t, cmd.Execute()) + assert.Contains(t, stderr.String(), "Claude Code databricks plugin v0.2.6 (up to date)") +} + +func TestUpdateMigratesManagedRawSkillsToPlugins(t *testing.T) { + setupTestAgents(t) + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.6") + resetUpdatePluginSeams(t) + + var rawUpdateAgents []string + origUpdateSkills := updateSkillsFn + t.Cleanup(func() { updateSkillsFn = origUpdateSkills }) + updateSkillsFn = func(_ context.Context, _ installer.ManifestSource, targetAgents []*agents.Agent, _ installer.UpdateOptions) (*installer.UpdateResult, error) { + for _, a := range targetAgents { + rawUpdateAgents = append(rawUpdateAgents, a.Name) + } + return &installer.UpdateResult{}, nil + } + updatePluginsFn = func(context.Context, string, string) ([]installer.PluginUpdate, error) { + return nil, nil + } + + var installed []pluginCall + updateInstallPluginForAgentFn = func(_ context.Context, a *agents.Agent, scope, ref string) (installer.PluginRecord, error) { + installed = append(installed, pluginCall{agent: a.Name, scope: scope}) + return installer.PluginRecord{ + Marketplace: a.Plugin.Marketplace, + Plugin: a.Plugin.ID, + Scope: scope, + Version: installer.DisplaySkillsVersion(ref), + }, nil + } + + var cleaned []string + updateCleanupLegacyFn = func(_ context.Context, a *agents.Agent, _ string) error { + cleaned = append(cleaned, a.Name) + return nil + } + updateRecordPluginInstallsFn = func(ctx context.Context, scope string, records map[string]installer.PluginRecord, ref string) error { + return installer.RecordPluginInstalls(ctx, scope, records, ref) + } + hasManagedRawSkillsForAgentFn = func(_ context.Context, a *agents.Agent, _ string) (bool, error) { + return a.Plugin != nil, nil + } + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + dir, err := installer.GlobalSkillsDir(ctx) + require.NoError(t, err) + require.NoError(t, installer.SaveState(dir, &installer.InstallState{ + SchemaVersion: 2, + Release: "v0.2.5", + Skills: map[string]string{"databricks": "0.2.5"}, + })) + + cmd := NewUpdateCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--scope", "global"}) + + require.NoError(t, cmd.Execute()) + + assert.Equal(t, []pluginCall{ + {agent: agents.NameClaudeCode, scope: agentScopeUser}, + {agent: agents.NameCodex, scope: agentScopeUser}, + {agent: agents.NameCopilot, scope: agentScopeUser}, + }, installed) + assert.Equal(t, []string{agents.NameClaudeCode, agents.NameCodex, agents.NameCopilot}, cleaned) + assert.Equal(t, []string{agents.NameCursor}, rawUpdateAgents) + assert.Contains(t, stderr.String(), "Installing databricks plugin for Claude Code...") + assert.Contains(t, stderr.String(), "GitHub Copilot databricks plugin v0.2.6") + + state, err := installer.LoadState(dir) + require.NoError(t, err) + require.NotNil(t, state) + assert.Contains(t, state.Plugins, agents.NameClaudeCode) + assert.Contains(t, state.Plugins, agents.NameCodex) + assert.Contains(t, state.Plugins, agents.NameCopilot) +} + +func TestUpdateKeepsRawSkillsWhenPluginMigrationStateWriteFails(t *testing.T) { + setupTestAgents(t) + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.6") + resetUpdatePluginSeams(t) + + origUpdateSkills := updateSkillsFn + t.Cleanup(func() { updateSkillsFn = origUpdateSkills }) + updateSkillsFn = func(context.Context, installer.ManifestSource, []*agents.Agent, installer.UpdateOptions) (*installer.UpdateResult, error) { + require.Fail(t, "raw skill update should not continue after migration state write fails") + return nil, nil + } + updatePluginsFn = func(context.Context, string, string) ([]installer.PluginUpdate, error) { + return nil, nil + } + + var installed []string + updateInstallPluginForAgentFn = func(_ context.Context, a *agents.Agent, scope, ref string) (installer.PluginRecord, error) { + installed = append(installed, a.Name) + return installer.PluginRecord{ + Marketplace: a.Plugin.Marketplace, + Plugin: a.Plugin.ID, + Scope: scope, + Version: installer.DisplaySkillsVersion(ref), + }, nil + } + + var cleaned []string + updateCleanupLegacyFn = func(_ context.Context, a *agents.Agent, _ string) error { + cleaned = append(cleaned, a.Name) + return nil + } + updateRecordPluginInstallsFn = func(context.Context, string, map[string]installer.PluginRecord, string) error { + return errors.New("state write failed") + } + hasManagedRawSkillsForAgentFn = func(_ context.Context, a *agents.Agent, _ string) (bool, error) { + return a.Plugin != nil, nil + } + + ctx := cmdio.MockDiscard(t.Context()) + dir, err := installer.GlobalSkillsDir(ctx) + require.NoError(t, err) + require.NoError(t, installer.SaveState(dir, &installer.InstallState{ + SchemaVersion: 2, + Release: "v0.2.5", + Skills: map[string]string{"databricks": "0.2.5"}, + })) + + cmd := NewUpdateCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--scope", "global"}) + + err = cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "state write failed") + assert.Equal(t, []string{agents.NameClaudeCode, agents.NameCodex, agents.NameCopilot}, installed) + assert.Empty(t, cleaned) + + state, err := installer.LoadState(dir) + require.NoError(t, err) + require.NotNil(t, state) + assert.Empty(t, state.Plugins) + assert.Equal(t, map[string]string{"databricks": "0.2.5"}, state.Skills) +} + +func TestUpdateCheckPrintsPluginMigrationPlan(t *testing.T) { + setupTestAgents(t) + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.6") + resetUpdatePluginSeams(t) + setupUpdateMock(t) + + updatePluginsFn = func(context.Context, string, string) ([]installer.PluginUpdate, error) { + require.Fail(t, "check mode must not run plugin update commands") + return nil, nil + } + updateInstallPluginForAgentFn = func(context.Context, *agents.Agent, string, string) (installer.PluginRecord, error) { + require.Fail(t, "check mode must not install plugins") + return installer.PluginRecord{}, nil + } + updateRecordPluginInstallsFn = func(context.Context, string, map[string]installer.PluginRecord, string) error { + require.Fail(t, "check mode must not record plugin installs") + return nil + } + updateCleanupLegacyFn = func(context.Context, *agents.Agent, string) error { + require.Fail(t, "check mode must not clean up legacy skills") + return nil + } + hasManagedRawSkillsForAgentFn = func(_ context.Context, a *agents.Agent, _ string) (bool, error) { + return a.Plugin != nil, nil + } + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + dir, err := installer.GlobalSkillsDir(ctx) + require.NoError(t, err) + require.NoError(t, installer.SaveState(dir, &installer.InstallState{ + SchemaVersion: 2, + Release: "v0.2.5", + Skills: map[string]string{"databricks": "0.2.5"}, + })) + + cmd := NewUpdateCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--scope", "global", "--check"}) + + require.NoError(t, cmd.Execute()) + assert.Contains(t, stderr.String(), "Claude Code would migrate raw skills to databricks plugin") + assert.Contains(t, stderr.String(), "Codex CLI would migrate raw skills to databricks plugin") + assert.Contains(t, stderr.String(), "GitHub Copilot would migrate raw skills to databricks plugin") + assert.NotContains(t, stderr.String(), "No changes.") +} + +func TestUpdateDoesNotMigratePluginAgentWithoutManagedRawSkills(t *testing.T) { + setupTestAgents(t) + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.6") + resetUpdatePluginSeams(t) + + var rawUpdateAgents []string + origUpdateSkills := updateSkillsFn + t.Cleanup(func() { updateSkillsFn = origUpdateSkills }) + updateSkillsFn = func(_ context.Context, _ installer.ManifestSource, targetAgents []*agents.Agent, _ installer.UpdateOptions) (*installer.UpdateResult, error) { + for _, a := range targetAgents { + rawUpdateAgents = append(rawUpdateAgents, a.Name) + } + return &installer.UpdateResult{}, nil + } + updatePluginsFn = func(context.Context, string, string) ([]installer.PluginUpdate, error) { + return nil, nil + } + updateInstallPluginForAgentFn = func(context.Context, *agents.Agent, string, string) (installer.PluginRecord, error) { + require.Fail(t, "must not migrate without managed raw skills") + return installer.PluginRecord{}, nil + } + updateRecordPluginInstallsFn = func(context.Context, string, map[string]installer.PluginRecord, string) error { + require.Fail(t, "must not record plugin installs without migration") + return nil + } + updateCleanupLegacyFn = func(context.Context, *agents.Agent, string) error { + require.Fail(t, "must not clean up without migration") + return nil + } + hasManagedRawSkillsForAgentFn = func(context.Context, *agents.Agent, string) (bool, error) { + return false, nil + } + + ctx := cmdio.MockDiscard(t.Context()) + dir, err := installer.GlobalSkillsDir(ctx) + require.NoError(t, err) + require.NoError(t, installer.SaveState(dir, &installer.InstallState{ + SchemaVersion: 2, + Release: "v0.2.5", + Skills: map[string]string{"databricks": "0.2.5"}, + })) + + cmd := NewUpdateCmd() + cmd.SetContext(ctx) + cmd.SetArgs([]string{"--scope", "global"}) + + require.NoError(t, cmd.Execute()) + assert.Equal(t, []string{agents.NameClaudeCode, agents.NameCursor}, rawUpdateAgents) +} + func TestUpdateNoPruneFlag(t *testing.T) { setupTestAgents(t) calls := setupUpdateMock(t) diff --git a/cmd/aitools/version.go b/cmd/aitools/version.go index 3244618a03..739174a186 100644 --- a/cmd/aitools/version.go +++ b/cmd/aitools/version.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/log" "github.com/spf13/cobra" + "golang.org/x/mod/semver" ) func NewVersionCmd() *cobra.Command { @@ -81,15 +82,25 @@ func NewVersionCmd() *cobra.Command { func printPluginLines(ctx context.Context, state *installer.InstallState, scope string) { for _, name := range slices.Sorted(maps.Keys(state.Plugins)) { rec := state.Plugins[name] - cmdio.LogString(ctx, fmt.Sprintf(" Plugin (%s, %s): %s", agentDisplayName(name), scope, versionToken(rec.Version))) + scopeLabel := scope + if rec.Scope != "" { + scopeLabel += ", " + rec.Scope + " scope" + } + cmdio.LogString(ctx, fmt.Sprintf(" Plugin (%s, %s): %s", agentDisplayName(name), scopeLabel, versionToken(rec.Version))) } } -// versionToken renders a skills/plugin version for output: the "latest" sentinel -// is shown as-is; a concrete version gets a leading "v". +// versionToken renders a skills/plugin version for output: the "latest" +// sentinel is explicit about tracking main; a concrete version gets a leading "v". func versionToken(v string) string { + if v == "" { + return "version unknown" + } if v == "latest" { - return "latest" + return "latest (tracking main)" + } + if !semver.IsValid("v" + v) { + return v } return "v" + v } diff --git a/cmd/aitools/version_test.go b/cmd/aitools/version_test.go index c314153f8d..cf4a6516e0 100644 --- a/cmd/aitools/version_test.go +++ b/cmd/aitools/version_test.go @@ -25,7 +25,7 @@ func TestVersionShowsPlugin(t *testing.T) { Release: "v0.2.6", LastUpdated: time.Date(2026, 6, 24, 0, 0, 0, 0, time.UTC), Plugins: map[string]installer.PluginRecord{ - "claude-code": {Marketplace: "databricks-agent-skills", Plugin: "databricks", Version: "0.2.6"}, + "claude-code": {Marketplace: "databricks-agent-skills", Plugin: "databricks", Scope: "user", Version: "0.2.6"}, }, })) @@ -34,7 +34,35 @@ func TestVersionShowsPlugin(t *testing.T) { cmd.SetContext(ctx) require.NoError(t, cmd.RunE(cmd, nil)) - assert.Contains(t, stderr.String(), "Plugin (Claude Code, global): v0.2.6") + assert.Contains(t, stderr.String(), "Plugin (Claude Code, global, user scope): v0.2.6") +} + +func TestVersionClarifiesLatest(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + t.Setenv("USERPROFILE", tmp) + t.Setenv("DATABRICKS_SKILLS_REF", "main") + t.Chdir(tmp) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + require.NoError(t, installer.SaveState(globalDir, &installer.InstallState{ + SchemaVersion: 2, + Release: "main", + LastUpdated: time.Date(2026, 6, 24, 0, 0, 0, 0, time.UTC), + Skills: map[string]string{"databricks-sql": "0.1.0"}, + Plugins: map[string]installer.PluginRecord{ + "claude-code": {Marketplace: "claude-plugins-official", Plugin: "databricks", Scope: "user", Version: "latest"}, + }, + })) + + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + cmd := NewVersionCmd() + cmd.SetContext(ctx) + require.NoError(t, cmd.RunE(cmd, nil)) + + output := stderr.String() + assert.Contains(t, output, "Skills (global): main") + assert.Contains(t, output, "Plugin (Claude Code, global, user scope): latest (tracking main)") } func TestVersionShowsBothScopes(t *testing.T) { diff --git a/libs/aitools/agents/agents.go b/libs/aitools/agents/agents.go index 4f9a4d082b..664288d138 100644 --- a/libs/aitools/agents/agents.go +++ b/libs/aitools/agents/agents.go @@ -124,7 +124,7 @@ const ( ) // databricksPlugin returns the shared plugin descriptor for an agent that -// installs from our own marketplace (Codex, Copilot, Cursor). +// installs from our own marketplace (Codex, Copilot). func databricksPlugin() *PluginSpec { return &PluginSpec{ Marketplace: databricksMarketplace, @@ -157,11 +157,9 @@ var Registry = []*Agent{ Plugin: claudePlugin(), }, { - Name: NameCursor, - DisplayName: "Cursor", - ConfigDir: homeSubdir(".cursor"), - SupportsProjectScope: true, - ProjectConfigDir: ".cursor", + Name: NameCursor, + DisplayName: "Cursor", + ConfigDir: homeSubdir(".cursor"), // Cursor's CLI binary is `cursor-agent`, not `cursor` (the latter is an // IDE shim that isn't on PATH unless the user ran "install shell command"). Binary: "cursor-agent", diff --git a/libs/aitools/installer/cleanup.go b/libs/aitools/installer/cleanup.go index 36c9bb4880..a307ef8547 100644 --- a/libs/aitools/installer/cleanup.go +++ b/libs/aitools/installer/cleanup.go @@ -18,48 +18,71 @@ import ( // the recorded checksums. User-modified or third-party directories, dirs with // extra files, and copies with no recorded provenance are left untouched. func RemoveLegacyRawSkills(ctx context.Context, agent *agents.Agent, scope string) error { - baseDir, err := skillsDir(ctx, scope) + entries, err := managedRawSkillEntriesForAgent(ctx, agent, scope) if err != nil { return err } + + for _, entryPath := range entries { + // RemoveAll on a symlink removes the link, not its target. + if err := os.RemoveAll(entryPath); err != nil { + log.Warnf(ctx, "Failed to remove legacy skill %s: %v", entryPath, err) + } else { + log.Debugf(ctx, "Removed legacy skill %s for %s", filepath.Base(entryPath), agent.DisplayName) + } + } + return nil +} + +// HasManagedRawSkillsForAgent reports whether an agent has raw skill exposure +// that this CLI can safely remove after migrating that agent to the plugin. +func HasManagedRawSkillsForAgent(ctx context.Context, agent *agents.Agent, scope string) (bool, error) { + entries, err := managedRawSkillEntriesForAgent(ctx, agent, scope) + if err != nil { + return false, err + } + return len(entries) > 0, nil +} + +func managedRawSkillEntriesForAgent(ctx context.Context, agent *agents.Agent, scope string) ([]string, error) { + baseDir, err := skillsDir(ctx, scope) + if err != nil { + return nil, err + } state, err := LoadState(baseDir) if err != nil { - return err + return nil, err } var cwd string if scope == ScopeProject { cwd, err = os.Getwd() if err != nil { - return err + return nil, err } } agentDir, err := agentSkillsDirForScope(ctx, agent, scope, cwd) if err != nil { - return err + return nil, err } entries, err := os.ReadDir(agentDir) if errors.Is(err, fs.ErrNotExist) { // No skills dir for this agent; nothing to clean up. - return nil + return nil, nil } if err != nil { - return err + return nil, err } + var managed []string for _, entry := range entries { entryPath := filepath.Join(agentDir, entry.Name()) canonicalDir := filepath.Join(baseDir, entry.Name()) if !exposureRemovable(entryPath, canonicalDir, entry.Name(), state) { continue } - // RemoveAll on a symlink removes the link, not its target. - if err := os.RemoveAll(entryPath); err != nil { - log.Warnf(ctx, "Failed to remove legacy skill %s: %v", entryPath, err) - } else { - log.Debugf(ctx, "Removed legacy skill %s for %s", entry.Name(), agent.DisplayName) - } + managed = append(managed, entryPath) } - return nil + return managed, nil } diff --git a/libs/aitools/installer/cleanup_test.go b/libs/aitools/installer/cleanup_test.go index 27cfc716c4..b19d627265 100644 --- a/libs/aitools/installer/cleanup_test.go +++ b/libs/aitools/installer/cleanup_test.go @@ -75,6 +75,46 @@ func TestRemoveLegacyRawSkills(t *testing.T) { assertExists(t, filepath.Join(agentDir, "delta")) } +func TestHasManagedRawSkillsForAgent(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + ctx := cmdio.MockDiscard(t.Context()) + + baseDir, err := GlobalSkillsDir(ctx) + require.NoError(t, err) + require.NoError(t, os.MkdirAll(filepath.Join(baseDir, "alpha"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(baseDir, "alpha", "SKILL.md"), []byte("alpha"), 0o644)) + + agentDir := filepath.Join(home, ".claude", "skills") + require.NoError(t, os.MkdirAll(agentDir, 0o755)) + require.NoError(t, os.Symlink(filepath.Join(baseDir, "alpha"), filepath.Join(agentDir, "alpha"))) + require.NoError(t, os.MkdirAll(filepath.Join(agentDir, "thirdparty"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(agentDir, "thirdparty", "SKILL.md"), []byte("mine"), 0o644)) + + require.NoError(t, SaveState(baseDir, &InstallState{ + SchemaVersion: schemaVersionV2, + Files: map[string]FileRecord{ + "alpha/SKILL.md": {SHA256: sha("alpha")}, + }, + })) + + agent := &agents.Agent{ + Name: agents.NameClaudeCode, + DisplayName: "Claude Code", + ConfigDir: func(_ context.Context) (string, error) { return filepath.Join(home, ".claude"), nil }, + } + + has, err := HasManagedRawSkillsForAgent(ctx, agent, ScopeGlobal) + require.NoError(t, err) + assert.True(t, has) + + require.NoError(t, RemoveLegacyRawSkills(ctx, agent, ScopeGlobal)) + has, err = HasManagedRawSkillsForAgent(ctx, agent, ScopeGlobal) + require.NoError(t, err) + assert.False(t, has) +} + func assertGone(t *testing.T, path string) { t.Helper() _, err := os.Lstat(path) diff --git a/libs/aitools/installer/installer_test.go b/libs/aitools/installer/installer_test.go index f32f737dc0..13c870cbeb 100644 --- a/libs/aitools/installer/installer_test.go +++ b/libs/aitools/installer/installer_test.go @@ -1039,7 +1039,7 @@ func TestInstallKeepsNameWhenRepoDirChanges(t *testing.T) { func TestSupportsProjectScopeSetCorrectly(t *testing.T) { expected := map[string]bool{ "claude-code": true, - "cursor": true, + "cursor": false, "codex": false, "opencode": false, "copilot": false, diff --git a/libs/aitools/installer/plugin_test.go b/libs/aitools/installer/plugin_test.go index 15415a6106..2652918c44 100644 --- a/libs/aitools/installer/plugin_test.go +++ b/libs/aitools/installer/plugin_test.go @@ -276,6 +276,39 @@ func TestUninstallSkillsOptsTearsDownPlugin(t *testing.T) { assert.Nil(t, state) } +func TestUninstallSkillsOptsTargetsPluginAgents(t *testing.T) { + setupTestHome(t) + stubAgentLookPath(t, true) + ctx, stub := process.WithStub(t.Context()) + stub.WithCallback(func(*exec.Cmd) error { return nil }) + ctx = cmdio.MockDiscard(ctx) + + dir, err := GlobalSkillsDir(ctx) + require.NoError(t, err) + require.NoError(t, SaveState(dir, &InstallState{ + SchemaVersion: schemaVersionV2, + Release: "v0.2.6", + Plugins: map[string]PluginRecord{ + agents.NameClaudeCode: {Marketplace: "claude-plugins-official", Plugin: "databricks", Scope: "user"}, + agents.NameCopilot: {Marketplace: "databricks-agent-skills", Plugin: "databricks", Scope: "user"}, + }, + })) + + require.NoError(t, UninstallSkillsOpts(ctx, UninstallOptions{Scope: ScopeGlobal, Agents: []string{agents.NameClaudeCode}})) + + cmds := stub.Commands() + assert.Contains(t, cmds, "claude plugin uninstall databricks@claude-plugins-official --scope user") + for _, cmd := range cmds { + assert.NotContains(t, cmd, "copilot plugin uninstall") + } + + state, err := LoadState(dir) + require.NoError(t, err) + require.NotNil(t, state) + assert.NotContains(t, state.Plugins, agents.NameClaudeCode) + assert.Contains(t, state.Plugins, agents.NameCopilot) +} + func TestUninstallNeverDeregistersBuiltinMarketplace(t *testing.T) { setupTestHome(t) stubAgentLookPath(t, true) diff --git a/libs/aitools/installer/uninstall.go b/libs/aitools/installer/uninstall.go index 2797373046..8207947499 100644 --- a/libs/aitools/installer/uninstall.go +++ b/libs/aitools/installer/uninstall.go @@ -19,6 +19,7 @@ import ( // UninstallOptions controls the behavior of UninstallSkillsOpts. type UninstallOptions struct { Skills []string // empty = all + Agents []string // empty = all agents Scope string // ScopeGlobal or ScopeProject (default: global) KeepMarketplace bool // keep the marketplace registration when removing a plugin } @@ -64,10 +65,15 @@ func UninstallSkillsOpts(ctx context.Context, opts UninstallOptions) error { // A full uninstall (no --skills filter) also tears down plugins. fullUninstall := len(opts.Skills) == 0 + agentFilter := stringSet(opts.Agents) + targetedAgents := len(agentFilter) > 0 pluginCount := 0 if fullUninstall { for _, name := range slices.Sorted(maps.Keys(state.Plugins)) { + if targetedAgents && !agentFilter[name] { + continue + } agent := agents.ByName(name) if agent == nil { // We can't tear down an agent we don't know; keep the record (and @@ -96,6 +102,11 @@ func UninstallSkillsOpts(ctx context.Context, opts UninstallOptions) error { seen := make(map[string]bool) for _, name := range opts.Skills { if _, ok := state.Skills[name]; !ok { + if targetedAgents { + if foundIn, found := untrackedSkillAgents(ctx, name, opts.Agents, scope, cwd); found { + return fmt.Errorf("skill %q is not tracked in Databricks aitools state but exists for %s. Run 'databricks aitools install --skills-only --agents %s' to rebuild state before uninstalling, or remove it manually", name, strings.Join(foundIn, ", "), strings.Join(opts.Agents, ",")) + } + } return fmt.Errorf("skill %q is not installed", name) } if seen[name] { @@ -113,7 +124,16 @@ func UninstallSkillsOpts(ctx context.Context, opts UninstallOptions) error { // Remove skill directories and symlinks for each skill. for _, name := range toRemove { canonicalDir := filepath.Join(baseDir, name) - removeSymlinksFromAgents(ctx, name, canonicalDir, scope, cwd) + removeCanonical := true + if targetedAgents { + removeSkillExposuresFromAgents(ctx, name, canonicalDir, scope, cwd, state, opts.Agents) + removeCanonical = !hasRemainingSkillExposure(ctx, name, canonicalDir, scope, cwd, opts.Agents) + } else { + removeSymlinksFromAgents(ctx, name, canonicalDir, scope, cwd) + } + if !removeCanonical { + continue + } if err := os.RemoveAll(canonicalDir); err != nil { log.Warnf(ctx, "Failed to remove %s: %v", canonicalDir, err) } @@ -157,6 +177,105 @@ func UninstallSkillsOpts(ctx context.Context, opts UninstallOptions) error { return nil } +func stringSet(items []string) map[string]bool { + if len(items) == 0 { + return nil + } + set := make(map[string]bool, len(items)) + for _, item := range items { + set[item] = true + } + return set +} + +func removeSkillExposuresFromAgents(ctx context.Context, skillName, canonicalDir, scope, cwd string, state *InstallState, agentNames []string) { + for _, name := range agentNames { + agent := agents.ByName(name) + if agent == nil { + continue + } + if scope == ScopeProject && !agent.SupportsProjectScope { + continue + } + agentDir, err := agentSkillsDirForScope(ctx, agent, scope, cwd) + if err != nil { + continue + } + entry := filepath.Join(agentDir, skillName) + if !exposureRemovable(entry, canonicalDir, skillName, state) { + log.Debugf(ctx, "Skipping unmanaged skill %s for %s", entry, agent.DisplayName) + continue + } + if err := os.RemoveAll(entry); err != nil { + log.Warnf(ctx, "Failed to remove %s: %v", entry, err) + } + } +} + +func hasRemainingSkillExposure(ctx context.Context, skillName, canonicalDir, scope, cwd string, removedAgents []string) bool { + removed := stringSet(removedAgents) + for _, agent := range agents.Registry { + if removed[agent.Name] { + continue + } + if scope == ScopeProject && !agent.SupportsProjectScope { + continue + } + agentDir, err := agentSkillsDirForScope(ctx, agent, scope, cwd) + if err != nil { + continue + } + entry := filepath.Join(agentDir, skillName) + if skillExposureExists(entry, canonicalDir) { + return true + } + } + return false +} + +func skillExposureExists(entry, canonicalDir string) bool { + fi, err := os.Lstat(entry) + if errors.Is(err, fs.ErrNotExist) { + return false + } + if err != nil { + return true + } + if fi.Mode()&os.ModeSymlink == 0 { + return true + } + target, err := os.Readlink(entry) + if err != nil { + return true + } + abs := target + if !filepath.IsAbs(target) { + abs = filepath.Clean(filepath.Join(filepath.Dir(entry), target)) + } + return abs == canonicalDir || strings.HasPrefix(abs, canonicalDir+string(os.PathSeparator)) +} + +func untrackedSkillAgents(ctx context.Context, skillName string, agentNames []string, scope, cwd string) ([]string, bool) { + var found []string + for _, name := range agentNames { + agent := agents.ByName(name) + if agent == nil { + continue + } + if scope == ScopeProject && !agent.SupportsProjectScope { + continue + } + agentDir, err := agentSkillsDirForScope(ctx, agent, scope, cwd) + if err != nil { + continue + } + if _, err := os.Lstat(filepath.Join(agentDir, skillName)); err == nil { + found = append(found, agent.DisplayName) + } + } + return found, len(found) > 0 +} + // removeSymlinksFromAgents removes a skill's symlink from all agent directories // in the registry, but only if the entry is a symlink pointing into canonicalDir. // Non-symlink directories are left untouched to avoid deleting user-managed content. diff --git a/libs/aitools/installer/uninstall_test.go b/libs/aitools/installer/uninstall_test.go index 7d483fdb4b..3bd7bfcc19 100644 --- a/libs/aitools/installer/uninstall_test.go +++ b/libs/aitools/installer/uninstall_test.go @@ -334,3 +334,76 @@ func TestUninstallSelectiveAllRemovesStateFile(t *testing.T) { _, err = os.Stat(filepath.Join(globalDir, ".state.json")) assert.ErrorIs(t, err, fs.ErrNotExist) } + +func TestUninstallTargetedAgentKeepsRemainingRawExposure(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".claude"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".cursor"), 0o755)) + + claudeAgent := agents.ByName(agents.NameClaudeCode) + cursorAgent := agents.ByName(agents.NameCursor) + src := &mockManifestSource{manifest: testManifest()} + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{claudeAgent, cursorAgent}, InstallOptions{})) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + cursorLink := filepath.Join(tmp, ".cursor", "skills", "databricks-sql") + claudeLink := filepath.Join(tmp, ".claude", "skills", "databricks-sql") + require.NoError(t, UninstallSkillsOpts(ctx, UninstallOptions{Agents: []string{agents.NameCursor}, Skills: []string{"databricks-sql"}})) + + _, err := os.Lstat(cursorLink) + assert.ErrorIs(t, err, fs.ErrNotExist) + _, err = os.Lstat(claudeLink) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(globalDir, "databricks-sql")) + assert.NoError(t, err) + + state, err := LoadState(globalDir) + require.NoError(t, err) + require.NotNil(t, state) + assert.Contains(t, state.Skills, "databricks-sql") +} + +func TestUninstallTargetedAgentRemovesCanonicalWhenNoExposureRemains(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".cursor"), 0o755)) + + cursorAgent := agents.ByName(agents.NameCursor) + src := &mockManifestSource{manifest: testManifest()} + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{cursorAgent}, InstallOptions{SpecificSkills: []string{"databricks-sql"}})) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + cursorDir := filepath.Join(tmp, ".cursor", "skills", "databricks-sql") + require.NoError(t, UninstallSkillsOpts(ctx, UninstallOptions{Agents: []string{agents.NameCursor}, Skills: []string{"databricks-sql"}})) + + _, err := os.Stat(cursorDir) + assert.ErrorIs(t, err, fs.ErrNotExist) + _, err = os.Stat(filepath.Join(globalDir, "databricks-sql")) + assert.ErrorIs(t, err, fs.ErrNotExist) + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.Nil(t, state) +} + +func TestUninstallTargetedUntrackedSkillGivesRecoveryGuidance(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + require.NoError(t, os.MkdirAll(filepath.Join(tmp, ".cursor", "skills", "databricks-sql"), 0o755)) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + require.NoError(t, SaveState(globalDir, &InstallState{ + SchemaVersion: schemaVersionV2, + Skills: map[string]string{"databricks-jobs": "0.1.0"}, + })) + + err := UninstallSkillsOpts(ctx, UninstallOptions{Agents: []string{agents.NameCursor}, Skills: []string{"databricks-sql"}}) + require.Error(t, err) + assert.Contains(t, err.Error(), "not tracked in Databricks aitools state") + assert.Contains(t, err.Error(), "Cursor") + assert.Contains(t, err.Error(), "install --skills-only --agents cursor") +}