diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index c58fd3ffb1..c07925559b 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -5,9 +5,7 @@ ### Notable Changes ### CLI - * Added `databricks aitools` command group for installing Databricks skills into your coding agents (Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity). Skills are fetched from [github.com/databricks/databricks-agent-skills](https://github.com/databricks/databricks-agent-skills) and either symlinked into each agent's skills directory or copied into the current project. Use `databricks aitools install` to set up, `update` to pull newer versions, `list` to see what's available, and `uninstall` to remove them. -* `[__settings__].default_profile` is now consulted as a fallback by `databricks api`, `databricks auth token`, and bundle commands when neither `--profile` nor `DATABRICKS_CONFIG_PROFILE` is set. `databricks auth token` continues to give precedence to `DATABRICKS_HOST` over `default_profile`. For bundle commands, `default_profile` only applies when the bundle does not pin its own `workspace.host`. ### Bundles * Make sure warnings asking for approval are understood by agents ([#5239](https://github.com/databricks/cli/pull/5239)) diff --git a/cmd/aitools/install.go b/cmd/aitools/install.go index 069de9bbec..ab51ad8f9c 100644 --- a/cmd/aitools/install.go +++ b/cmd/aitools/install.go @@ -51,7 +51,7 @@ func defaultPromptAgentSelection(ctx context.Context, detected []*agents.Agent) } func NewInstallCmd() *cobra.Command { - var skillsFlag, agentsFlag string + var skillsFlag, agentsFlag, scopeFlag string var includeExperimental bool var projectFlag, globalFlag bool @@ -61,17 +61,30 @@ func NewInstallCmd() *cobra.Command { Long: `Install Databricks AI skills for detected coding agents. By default, skills are installed globally to each agent's skills directory. -Use --project to install to the current project directory instead. +Use --scope=project to install to the current project directory instead. When multiple agents are detected, skills are stored in a canonical location and symlinked to each agent to avoid duplication. Use --skills name1,name2 to install specific skills. +Agent selection: + --agents [,...] Install only for the named agents. + (unset, interactive) Multi-select prompt over detected agents. + (unset, non-interactive) Install for every detected agent. + +The list of agents the command will act on is always logged to stderr before +the install runs, so callers can verify what was picked. + Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() + projectFlag, globalFlag, err := parseScopeFlag(scopeFlag, projectFlag, globalFlag, false) + if err != nil { + return err + } + // Resolve scope. scope, err := resolveScopeWithPrompt(ctx, projectFlag, globalFlag) if err != nil { @@ -130,8 +143,10 @@ Supported agents: Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Anti cmd.Flags().StringVar(&skillsFlag, "skills", "", "Specific skills to install (comma-separated)") cmd.Flags().StringVar(&agentsFlag, "agents", "", "Agents to install for (comma-separated, e.g. claude-code,cursor)") cmd.Flags().BoolVar(&includeExperimental, "experimental", false, "Include experimental skills") + cmd.Flags().StringVar(&scopeFlag, "scope", "", "Install scope: project or global (default: global, or prompt when interactive)") cmd.Flags().BoolVar(&projectFlag, "project", false, "Install to project directory (cwd)") cmd.Flags().BoolVar(&globalFlag, "global", false, "Install globally (default)") + markScopeBoolsDeprecated(cmd) return cmd } diff --git a/cmd/aitools/install_test.go b/cmd/aitools/install_test.go index 0ed99bcbfd..22e57ee0aa 100644 --- a/cmd/aitools/install_test.go +++ b/cmd/aitools/install_test.go @@ -411,6 +411,45 @@ func TestInstallGlobalAndProjectErrors(t *testing.T) { assert.Contains(t, err.Error(), "cannot use --global and --project together") } +func TestInstallScopeFlag(t *testing.T) { + tests := []struct { + name string + args []string + wantScope string + wantErr string + }{ + {name: "scope project", args: []string{"--scope", "project"}, wantScope: installer.ScopeProject}, + {name: "scope global", args: []string{"--scope", "global"}, wantScope: installer.ScopeGlobal}, + {name: "scope both rejected", args: []string{"--scope", "both"}, wantErr: "--scope=both is not supported"}, + {name: "scope invalid value", args: []string{"--scope", "all"}, wantErr: `invalid --scope "all"`}, + {name: "scope conflicts with legacy", args: []string{"--scope", "global", "--project"}, wantErr: "cannot use --scope with --project or --global"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setupTestAgents(t) + calls := setupInstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := NewInstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs(tt.args) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + err := cmd.Execute() + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + require.Len(t, *calls, 1) + assert.Equal(t, tt.wantScope, (*calls)[0].opts.Scope) + }) + } +} + func TestInstallNoFlagNonInteractiveUsesGlobal(t *testing.T) { setupTestAgents(t) calls := setupInstallMock(t) diff --git a/cmd/aitools/list.go b/cmd/aitools/list.go index bb9a01b3bd..62d17f3664 100644 --- a/cmd/aitools/list.go +++ b/cmd/aitools/list.go @@ -19,6 +19,7 @@ import ( var listSkillsFn = defaultListSkills func NewListCmd() *cobra.Command { + var scopeFlag string var projectFlag, globalFlag bool cmd := &cobra.Command{ @@ -26,22 +27,34 @@ func NewListCmd() *cobra.Command { Short: "List installed AI tools components", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - if projectFlag && globalFlag { + // Reject the legacy --project --global combination here so it + // doesn't silently degrade to --scope=both. Users who want both + // scopes should use --scope=both (the new explicit spelling). + if projectFlag && globalFlag && scopeFlag == "" { return errors.New("cannot use --global and --project together") } - // For list: no flag = show both scopes (empty string). + + projectFlag, globalFlag, err := parseScopeFlag(scopeFlag, projectFlag, globalFlag, true) + if err != nil { + return err + } + + // list: empty scope = show both. --scope=both also lands here. var scope string - if projectFlag { + switch { + case projectFlag && !globalFlag: scope = installer.ScopeProject - } else if globalFlag { + case globalFlag && !projectFlag: scope = installer.ScopeGlobal } return listSkillsFn(cmd, scope) }, } + cmd.Flags().StringVar(&scopeFlag, "scope", "", "Scope to show: project, global, or both (default: both)") cmd.Flags().BoolVar(&projectFlag, "project", false, "Show only project-scoped skills") cmd.Flags().BoolVar(&globalFlag, "global", false, "Show only globally-scoped skills") + markScopeBoolsDeprecated(cmd) return cmd } diff --git a/cmd/aitools/list_test.go b/cmd/aitools/list_test.go index cb4ac9d9db..5260ad5169 100644 --- a/cmd/aitools/list_test.go +++ b/cmd/aitools/list_test.go @@ -3,6 +3,7 @@ package aitools import ( "testing" + "github.com/databricks/cli/libs/aitools/installer" "github.com/databricks/cli/libs/cmdio" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -36,7 +37,58 @@ func TestListCommandCallsListFn(t *testing.T) { func TestListCommandHasScopeFlags(t *testing.T) { cmd := NewListCmd() f := cmd.Flags().Lookup("project") - require.NotNil(t, f, "--project flag should exist") + require.NotNil(t, f, "--project flag should exist (deprecated alias)") + assert.NotEmpty(t, f.Deprecated, "--project should be marked deprecated") f = cmd.Flags().Lookup("global") - require.NotNil(t, f, "--global flag should exist") + require.NotNil(t, f, "--global flag should exist (deprecated alias)") + assert.NotEmpty(t, f.Deprecated, "--global should be marked deprecated") + f = cmd.Flags().Lookup("scope") + require.NotNil(t, f, "--scope flag should exist") +} + +func TestListScopeFlag(t *testing.T) { + tests := []struct { + name string + args []string + wantScope string + wantErr string + }{ + {name: "scope project", args: []string{"--scope", "project"}, wantScope: installer.ScopeProject}, + {name: "scope global", args: []string{"--scope", "global"}, wantScope: installer.ScopeGlobal}, + {name: "scope both shows both", args: []string{"--scope", "both"}, wantScope: ""}, + {name: "scope invalid", args: []string{"--scope", "all"}, wantErr: `invalid --scope "all"`}, + {name: "legacy both flags together rejected", args: []string{"--project", "--global"}, wantErr: "cannot use --global and --project together"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + orig := listSkillsFn + t.Cleanup(func() { listSkillsFn = orig }) + + var gotScope string + called := false + listSkillsFn = func(_ *cobra.Command, scope string) error { + called = true + gotScope = scope + return nil + } + + ctx := cmdio.MockDiscard(t.Context()) + cmd := NewListCmd() + cmd.SetContext(ctx) + cmd.SetArgs(tt.args) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + err := cmd.Execute() + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + assert.True(t, called) + assert.Equal(t, tt.wantScope, gotScope) + }) + } } diff --git a/cmd/aitools/scope.go b/cmd/aitools/scope.go index 3679b46411..15f0082539 100644 --- a/cmd/aitools/scope.go +++ b/cmd/aitools/scope.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/libs/aitools/installer" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/env" + "github.com/spf13/cobra" ) // promptScopeSelection is a package-level var so tests can replace it with a mock. @@ -82,6 +83,53 @@ func defaultPromptScopeSelection(ctx context.Context) (string, error) { const scopeBoth = "both" +// markScopeBoolsDeprecated hides --project and --global from help and emits a +// stderr warning pointing at --scope when they're used. The booleans are kept +// so existing scripts and the experimental backward-compat aliases keep +// working through the next release. +func markScopeBoolsDeprecated(cmd *cobra.Command) { + cmd.Flags().Lookup("project").Deprecated = "use --scope=project" + cmd.Flags().Lookup("project").Hidden = true + cmd.Flags().Lookup("global").Deprecated = "use --scope=global" + cmd.Flags().Lookup("global").Hidden = true +} + +// parseScopeFlag translates --scope into the equivalent --project/--global bool pair. +// Returns (projectFlag, globalFlag, nil) unchanged when --scope is empty so the +// deprecated booleans can keep flowing through the existing resolveScope* helpers +// (including update's supported `--project --global` "both scopes" path). Errors +// if --scope is combined with --project or --global. When allowBoth is false, +// --scope=both is rejected up front so install and uninstall don't have to +// special-case it. +// +// Note: install/list/uninstall reject the legacy `--project --global` combination +// at their own RunE / resolveScope layer; update intentionally accepts it as the +// "both scopes" path until those flags are removed. +func parseScopeFlag(scopeFlag string, projectFlag, globalFlag, allowBoth bool) (proj, glob bool, err error) { + if scopeFlag == "" { + return projectFlag, globalFlag, nil + } + if projectFlag || globalFlag { + return false, false, errors.New("cannot use --scope with --project or --global; --project and --global are deprecated aliases for --scope") + } + switch scopeFlag { + case installer.ScopeProject: + return true, false, nil + case installer.ScopeGlobal: + return false, true, nil + case scopeBoth: + if !allowBoth { + return false, false, errors.New("--scope=both is not supported for this command; use 'project' or 'global'") + } + return true, true, nil + default: + if allowBoth { + return false, false, fmt.Errorf("invalid --scope %q: must be one of project, global, both", scopeFlag) + } + return false, false, fmt.Errorf("invalid --scope %q: must be one of project, global", scopeFlag) + } +} + // detectInstalledScopes checks which scopes have a .state.json file present. func detectInstalledScopes(globalDir, projectDir string) (global, project bool, err error) { globalState, err := installer.LoadState(globalDir) @@ -132,7 +180,7 @@ func resolveScopeForUpdate(ctx context.Context, projectFlag, globalFlag bool, gl switch { case hasGlobal && hasProject: if !cmdio.IsPromptSupported(ctx) { - return nil, errors.New("skills are installed in both global and project scopes; use --global, --project, or both flags to specify which to update") + return nil, errors.New("skills are installed in both global and project scopes; use --scope=global, --scope=project, or --scope=both to specify which to update") } scopes, err := promptUpdateScopeSelection(ctx) if err != nil { @@ -158,7 +206,7 @@ func resolveScopeForUpdate(ctx context.Context, projectFlag, globalFlag bool, gl // Unlike update, uninstall never allows "both" scopes at once. func resolveScopeForUninstall(ctx context.Context, projectFlag, globalFlag bool, globalDir, projectDir string) (string, error) { if projectFlag && globalFlag { - return "", errors.New("cannot uninstall both scopes at once; run uninstall separately for --global and --project") + return "", errors.New("cannot uninstall both scopes at once; run uninstall separately with --scope=global and --scope=project") } hasGlobal, hasProject, err := detectInstalledScopes(globalDir, projectDir) @@ -182,7 +230,7 @@ func resolveScopeForUninstall(ctx context.Context, projectFlag, globalFlag bool, switch { case hasGlobal && hasProject: if !cmdio.IsPromptSupported(ctx) { - return "", errors.New("skills are installed in both global and project scopes; use --global or --project to specify which to uninstall") + return "", errors.New("skills are installed in both global and project scopes; use --scope=global or --scope=project to specify which to uninstall") } scope, err := promptUninstallScopeSelection(ctx) if err != nil { @@ -230,10 +278,10 @@ func scopeNotInstalledError(scope, verb, projectDir string, hasGlobal, hasProjec "no project-scoped skills found in the current directory.\n\n"+ "Project-scoped skills are detected based on your working directory.\n"+ "Make sure you are in the project root where you originally ran\n"+ - "'databricks aitools install --project'.\n\n"+ + "'databricks aitools install --scope=project'.\n\n"+ "Expected location: %s/", expectedPath) } else { - msg = "no globally-scoped skills installed. Run 'databricks aitools install --global' to install" + msg = "no globally-scoped skills installed. Run 'databricks aitools install --scope=global' to install" } hint := crossScopeHint(scope, verb, hasGlobal, hasProject) @@ -248,10 +296,10 @@ func scopeNotInstalledError(scope, verb, projectDir string, hasGlobal, hasProjec // The verb parameter (e.g. "update", "uninstall") controls the action in the hint message. func crossScopeHint(requestedScope, verb string, hasGlobal, hasProject bool) string { if requestedScope == installer.ScopeProject && hasGlobal { - return fmt.Sprintf("Global skills are installed. Run without --project to %s those.", verb) + return fmt.Sprintf("Global skills are installed. Run with --scope=global to %s those.", verb) } if requestedScope == installer.ScopeGlobal && hasProject { - return fmt.Sprintf("Project-scoped skills are installed. Run without --global to %s those.", verb) + return fmt.Sprintf("Project-scoped skills are installed. Run with --scope=project to %s those.", verb) } return "" } diff --git a/cmd/aitools/scope_test.go b/cmd/aitools/scope_test.go index 371e91264c..da941dbb31 100644 --- a/cmd/aitools/scope_test.go +++ b/cmd/aitools/scope_test.go @@ -67,6 +67,57 @@ func interactiveCtx(t *testing.T) (context.Context, func()) { return ctx, test.Done } +// --- parseScopeFlag tests --- + +func TestParseScopeFlag(t *testing.T) { + tests := []struct { + name string + scope string + project bool + global bool + allowBoth bool + wantProj bool + wantGlob bool + wantErr string + }{ + {name: "unset", scope: ""}, + {name: "legacy project only", project: true, wantProj: true}, + {name: "legacy global only", global: true, wantGlob: true}, + {name: "legacy both passthrough", project: true, global: true, wantProj: true, wantGlob: true}, + {name: "scope project", scope: "project", wantProj: true}, + {name: "scope global", scope: "global", wantGlob: true}, + {name: "scope both allowed", scope: "both", allowBoth: true, wantProj: true, wantGlob: true}, + {name: "scope both disallowed", scope: "both", wantErr: "--scope=both is not supported"}, + {name: "scope invalid value with allowBoth", scope: "all", allowBoth: true, wantErr: `invalid --scope "all": must be one of project, global, both`}, + {name: "scope invalid value without allowBoth omits both from error", scope: "all", wantErr: `invalid --scope "all": must be one of project, global`}, + {name: "scope conflicts with project", scope: "project", project: true, wantErr: "cannot use --scope with --project or --global"}, + {name: "scope conflicts with global", scope: "global", global: true, wantErr: "cannot use --scope with --project or --global"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + proj, glob, err := parseScopeFlag(tt.scope, tt.project, tt.global, tt.allowBoth) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantProj, proj) + assert.Equal(t, tt.wantGlob, glob) + }) + } + + // Stronger check that the without-allowBoth invalid-value branch omits + // "both" from the error message (the table assertion uses Contains which + // can't distinguish a substring shared with the allowBoth variant). + t.Run("invalid scope error message without allowBoth does not mention both", func(t *testing.T) { + _, _, err := parseScopeFlag("all", false, false, false) + require.Error(t, err) + assert.NotContains(t, err.Error(), "both") + }) +} + // --- detectInstalledScopes tests (table-driven) --- func TestDetectInstalledScopes(t *testing.T) { @@ -184,21 +235,21 @@ func TestCrossScopeHint(t *testing.T) { scope: installer.ScopeProject, verb: "update", hasGlobal: true, - wantHint: "without --project to update those", + wantHint: "with --scope=global to update those", }, { name: "ProjectMissingHintsGlobalUninstall", scope: installer.ScopeProject, verb: "uninstall", hasGlobal: true, - wantHint: "without --project to uninstall those", + wantHint: "with --scope=global to uninstall those", }, { name: "GlobalMissingHintsProject", scope: installer.ScopeGlobal, verb: "update", hasProj: true, - wantHint: "without --global to update those", + wantHint: "with --scope=project to update those", }, { name: "NeitherInstalledNoHint", @@ -225,7 +276,7 @@ func TestScopeNotInstalledErrorProjectIncludesPath(t *testing.T) { projectDir := "/some/project/.databricks/aitools/skills" err := scopeNotInstalledError(installer.ScopeProject, "update", projectDir, false, false) assert.Contains(t, err.Error(), "no project-scoped skills found") - assert.Contains(t, err.Error(), "install --project") + assert.Contains(t, err.Error(), "install --scope=project") assert.Contains(t, err.Error(), "Expected location:") assert.Contains(t, err.Error(), "/some/project/.databricks/aitools/skills/") } @@ -233,7 +284,7 @@ func TestScopeNotInstalledErrorProjectIncludesPath(t *testing.T) { func TestScopeNotInstalledErrorGlobal(t *testing.T) { err := scopeNotInstalledError(installer.ScopeGlobal, "update", "/irrelevant", false, false) assert.Contains(t, err.Error(), "no globally-scoped skills installed") - assert.Contains(t, err.Error(), "install --global") + assert.Contains(t, err.Error(), "install --scope=global") } // --- resolveScopeForUpdate tests --- @@ -307,7 +358,7 @@ func TestResolveScopeForUpdateProjectFlagNoInstall(t *testing.T) { _, err := resolveScopeForUpdate(ctx, true, false, globalDir, projectDir) require.Error(t, err) assert.Contains(t, err.Error(), "no project-scoped skills found") - assert.Contains(t, err.Error(), "install --project") + assert.Contains(t, err.Error(), "install --scope=project") assert.Contains(t, err.Error(), "Expected location:") assert.Contains(t, err.Error(), ".databricks/aitools/skills/") } @@ -351,8 +402,8 @@ func TestResolveScopeForUpdateNoFlagsBothNonInteractive(t *testing.T) { _, err := resolveScopeForUpdate(ctx, false, false, globalDir, projectDir) require.Error(t, err) assert.Contains(t, err.Error(), "skills are installed in both global and project scopes") - assert.Contains(t, err.Error(), "--global") - assert.Contains(t, err.Error(), "--project") + assert.Contains(t, err.Error(), "--scope=global") + assert.Contains(t, err.Error(), "--scope=project") } func TestResolveScopeForUpdateNoFlagsBothInteractive(t *testing.T) { @@ -425,7 +476,7 @@ func TestResolveScopeForUninstallProjectFlagNoInstall(t *testing.T) { _, err := resolveScopeForUninstall(ctx, true, false, globalDir, projectDir) require.Error(t, err) assert.Contains(t, err.Error(), "no project-scoped skills found") - assert.Contains(t, err.Error(), "install --project") + assert.Contains(t, err.Error(), "install --scope=project") assert.Contains(t, err.Error(), "Expected location:") } @@ -468,8 +519,8 @@ func TestResolveScopeForUninstallNoFlagsBothNonInteractive(t *testing.T) { _, err := resolveScopeForUninstall(ctx, false, false, globalDir, projectDir) require.Error(t, err) assert.Contains(t, err.Error(), "skills are installed in both global and project scopes") - assert.Contains(t, err.Error(), "--global") - assert.Contains(t, err.Error(), "--project") + assert.Contains(t, err.Error(), "--scope=global") + assert.Contains(t, err.Error(), "--scope=project") } func TestResolveScopeForUninstallNoFlagsBothInteractive(t *testing.T) { @@ -556,7 +607,7 @@ func TestResolveScopeForUninstallProjectFlagHintsUninstall(t *testing.T) { // Project flag with no project state should hint about global using "uninstall" verb. _, err := resolveScopeForUninstall(ctx, true, false, globalDir, projectDir) require.Error(t, err) - assert.Contains(t, err.Error(), "without --project to uninstall those") + assert.Contains(t, err.Error(), "with --scope=global to uninstall those") } func TestResolveScopeForUpdateProjectFlagHintsUpdate(t *testing.T) { @@ -567,5 +618,5 @@ func TestResolveScopeForUpdateProjectFlagHintsUpdate(t *testing.T) { // Project flag with no project state should hint about global using "update" verb. _, err := resolveScopeForUpdate(ctx, true, false, globalDir, projectDir) require.Error(t, err) - assert.Contains(t, err.Error(), "without --project to update those") + assert.Contains(t, err.Error(), "with --scope=global to update those") } diff --git a/cmd/aitools/uninstall.go b/cmd/aitools/uninstall.go index a13fc6de4d..7f4b32ebf6 100644 --- a/cmd/aitools/uninstall.go +++ b/cmd/aitools/uninstall.go @@ -1,12 +1,19 @@ package aitools import ( + "context" + "github.com/databricks/cli/libs/aitools/installer" "github.com/spf13/cobra" ) +// Package-level for testability. Tests override via uninstall_test.go. +var uninstallSkillsFn = func(ctx context.Context, opts installer.UninstallOptions) error { + return installer.UninstallSkillsOpts(ctx, opts) +} + func NewUninstallCmd() *cobra.Command { - var skillsFlag string + var skillsFlag, scopeFlag string var projectFlag, globalFlag bool cmd := &cobra.Command{ @@ -19,6 +26,11 @@ By default, removes all skills. Use --skills to remove specific skills only.`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() + projectFlag, globalFlag, err := parseScopeFlag(scopeFlag, projectFlag, globalFlag, false) + if err != nil { + return err + } + globalDir, err := installer.GlobalSkillsDir(ctx) if err != nil { return err @@ -37,12 +49,14 @@ By default, removes all skills. Use --skills to remove specific skills only.`, Scope: scope, } opts.Skills = splitAndTrim(skillsFlag) - return installer.UninstallSkillsOpts(ctx, opts) + return uninstallSkillsFn(ctx, opts) }, } cmd.Flags().StringVar(&skillsFlag, "skills", "", "Specific skills to uninstall (comma-separated)") + cmd.Flags().StringVar(&scopeFlag, "scope", "", "Uninstall scope: project or global") cmd.Flags().BoolVar(&projectFlag, "project", false, "Uninstall project-scoped skills") cmd.Flags().BoolVar(&globalFlag, "global", false, "Uninstall globally-scoped skills") + markScopeBoolsDeprecated(cmd) return cmd } diff --git a/cmd/aitools/uninstall_test.go b/cmd/aitools/uninstall_test.go new file mode 100644 index 0000000000..ec0dff7989 --- /dev/null +++ b/cmd/aitools/uninstall_test.go @@ -0,0 +1,66 @@ +package aitools + +import ( + "context" + "testing" + + "github.com/databricks/cli/libs/aitools/installer" + "github.com/databricks/cli/libs/cmdio" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func setupUninstallMock(t *testing.T) *[]installer.UninstallOptions { + t.Helper() + orig := uninstallSkillsFn + t.Cleanup(func() { uninstallSkillsFn = orig }) + + var calls []installer.UninstallOptions + uninstallSkillsFn = func(_ context.Context, opts installer.UninstallOptions) error { + calls = append(calls, opts) + return nil + } + return &calls +} + +func TestUninstallScopeFlag(t *testing.T) { + tests := []struct { + name string + args []string + wantScope string + wantErr string + }{ + // scope=project requires installed project state and is covered via TestParseScopeFlag + // and TestResolveScopeForUninstallProjectFlagWithState. Here we cover the no-state paths + // and the failure modes specific to the Cobra wiring. + {name: "scope global", args: []string{"--scope", "global"}, wantScope: installer.ScopeGlobal}, + {name: "scope both rejected", args: []string{"--scope", "both"}, wantErr: "--scope=both is not supported"}, + {name: "scope invalid value", args: []string{"--scope", "all"}, wantErr: `invalid --scope "all"`}, + {name: "scope conflicts with legacy project", args: []string{"--scope", "global", "--project"}, wantErr: "cannot use --scope with --project or --global"}, + {name: "legacy both flags together rejected", args: []string{"--project", "--global"}, wantErr: "cannot uninstall both scopes at once"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setupTestAgents(t) + calls := setupUninstallMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := NewUninstallCmd() + cmd.SetContext(ctx) + cmd.SetArgs(tt.args) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + err := cmd.Execute() + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + require.Len(t, *calls, 1) + assert.Equal(t, tt.wantScope, (*calls)[0].Scope) + }) + } +} diff --git a/cmd/aitools/update.go b/cmd/aitools/update.go index 7979c29f3b..4452440c2e 100644 --- a/cmd/aitools/update.go +++ b/cmd/aitools/update.go @@ -1,6 +1,7 @@ package aitools import ( + "context" "fmt" "github.com/databricks/cli/libs/aitools/agents" @@ -9,9 +10,14 @@ import ( "github.com/spf13/cobra" ) +// Package-level for testability. Tests override via update_test.go. +var updateSkillsFn = func(ctx context.Context, src installer.ManifestSource, installed []*agents.Agent, opts installer.UpdateOptions) (*installer.UpdateResult, error) { + return installer.UpdateSkills(ctx, src, installed, opts) +} + func NewUpdateCmd() *cobra.Command { var check, force, noNew bool - var skillsFlag string + var skillsFlag, scopeFlag string var projectFlag, globalFlag bool cmd := &cobra.Command{ @@ -35,6 +41,11 @@ preview what would change without downloading.`, return err } + projectFlag, globalFlag, err := parseScopeFlag(scopeFlag, projectFlag, globalFlag, true) + if err != nil { + return err + } + scopes, err := resolveScopeForUpdate(ctx, projectFlag, globalFlag, globalDir, projectDir) if err != nil { return err @@ -57,7 +68,7 @@ preview what would change without downloading.`, } opts.Skills = skills - result, err := installer.UpdateSkills(ctx, src, installed, opts) + result, err := updateSkillsFn(ctx, src, installed, opts) if err != nil { return err } @@ -73,7 +84,9 @@ preview what would change without downloading.`, cmd.Flags().BoolVar(&force, "force", false, "Re-download even if versions match") cmd.Flags().BoolVar(&noNew, "no-new", false, "Don't auto-install new skills from manifest") cmd.Flags().StringVar(&skillsFlag, "skills", "", "Specific skills to update (comma-separated)") + cmd.Flags().StringVar(&scopeFlag, "scope", "", "Update scope: project, global, or both") cmd.Flags().BoolVar(&projectFlag, "project", false, "Update project-scoped skills") cmd.Flags().BoolVar(&globalFlag, "global", false, "Update globally-scoped skills") + markScopeBoolsDeprecated(cmd) return cmd } diff --git a/cmd/aitools/update_test.go b/cmd/aitools/update_test.go new file mode 100644 index 0000000000..f93094a81b --- /dev/null +++ b/cmd/aitools/update_test.go @@ -0,0 +1,72 @@ +package aitools + +import ( + "context" + "testing" + + "github.com/databricks/cli/libs/aitools/agents" + "github.com/databricks/cli/libs/aitools/installer" + "github.com/databricks/cli/libs/cmdio" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func setupUpdateMock(t *testing.T) *[]installer.UpdateOptions { + t.Helper() + orig := updateSkillsFn + t.Cleanup(func() { updateSkillsFn = orig }) + + var calls []installer.UpdateOptions + updateSkillsFn = func(_ context.Context, _ installer.ManifestSource, _ []*agents.Agent, opts installer.UpdateOptions) (*installer.UpdateResult, error) { + calls = append(calls, opts) + return &installer.UpdateResult{}, nil + } + return &calls +} + +func TestUpdateScopeFlag(t *testing.T) { + tests := []struct { + name string + args []string + wantScopes []string + wantErr string + }{ + // scope=project requires installed project state and is covered via TestParseScopeFlag + // (cmd/aitools/scope_test.go) and TestResolveScopeForUpdateProjectFlagWithState. Here + // we cover the no-state paths and the failure modes specific to the Cobra wiring. + {name: "scope global", args: []string{"--scope", "global"}, wantScopes: []string{installer.ScopeGlobal}}, + {name: "scope both with no installs falls through to global", args: []string{"--scope", "both"}, wantScopes: []string{installer.ScopeGlobal}}, + {name: "scope invalid value", args: []string{"--scope", "all"}, wantErr: `invalid --scope "all"`}, + {name: "scope conflicts with legacy project", args: []string{"--scope", "global", "--project"}, wantErr: "cannot use --scope with --project or --global"}, + // Legacy `--project --global` is the supported "update both scopes" path + // (preserved until the deprecated flags are removed). Without state, it + // falls through to global per TestResolveScopeForUpdateBothFlagsNeitherInstalled. + {name: "legacy both flags fall through to global without state", args: []string{"--project", "--global"}, wantScopes: []string{installer.ScopeGlobal}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setupTestAgents(t) + calls := setupUpdateMock(t) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := NewUpdateCmd() + cmd.SetContext(ctx) + cmd.SetArgs(tt.args) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + + err := cmd.Execute() + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + require.Len(t, *calls, len(tt.wantScopes)) + for i, scope := range tt.wantScopes { + assert.Equal(t, scope, (*calls)[i].Scope) + } + }) + } +}