diff --git a/experimental/aitools/cmd/scope.go b/experimental/aitools/cmd/scope.go index 98464d9b1f..8c6ce0f013 100644 --- a/experimental/aitools/cmd/scope.go +++ b/experimental/aitools/cmd/scope.go @@ -3,6 +3,7 @@ package aitools import ( "context" "errors" + "fmt" "os" "path/filepath" @@ -15,6 +16,12 @@ import ( // promptScopeSelection is a package-level var so tests can replace it with a mock. var promptScopeSelection = defaultPromptScopeSelection +// promptUpdateScopeSelection is a package-level var for the update scope prompt (3 options: global/project/both). +var promptUpdateScopeSelection = defaultPromptUpdateScopeSelection + +// promptUninstallScopeSelection is a package-level var for the uninstall scope prompt (2 options: global/project). +var promptUninstallScopeSelection = defaultPromptUninstallScopeSelection + // resolveScope validates --project and --global flags and returns the scope. func resolveScope(project, global bool) (string, error) { if project && global { @@ -72,3 +79,248 @@ func defaultPromptScopeSelection(ctx context.Context) (string, error) { return scope, nil } + +const scopeBoth = "both" + +// 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) + if err != nil { + return false, false, err + } + + projectState, err := installer.LoadState(projectDir) + if err != nil { + return false, false, err + } + + return globalState != nil, projectState != nil, nil +} + +// resolveScopeForUpdate resolves scopes for the update command. +// Returns one or more scopes to update. When both flags are set, global always passes through +// (for legacy install detection) and project is checked via state. +func resolveScopeForUpdate(ctx context.Context, projectFlag, globalFlag bool, globalDir, projectDir string) ([]string, error) { + hasGlobal, hasProject, err := detectInstalledScopes(globalDir, projectDir) + if err != nil { + return nil, err + } + + if projectFlag && globalFlag { + var scopes []string + if hasGlobal { + scopes = append(scopes, installer.ScopeGlobal) + } + if hasProject { + scopes = append(scopes, installer.ScopeProject) + } + if len(scopes) == 0 { + // Neither installed. Fall through to global for legacy detection. + return []string{installer.ScopeGlobal}, nil + } + return scopes, nil + } + if projectFlag { + return withExplicitScopeCheck(projectDir, installer.ScopeProject, "update", projectDir, hasGlobal, hasProject) + } + if globalFlag { + // Always pass through to the installer layer, which handles legacy installs. + return []string{installer.ScopeGlobal}, nil + } + + // No flags: auto-detect. + 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") + } + scopes, err := promptUpdateScopeSelection(ctx) + if err != nil { + return nil, err + } + return scopes, nil + + case hasGlobal: + return []string{installer.ScopeGlobal}, nil + + case hasProject: + return []string{installer.ScopeProject}, nil + + default: + // Fall through to global scope so the installer layer can detect + // legacy installs (skills on disk without .state.json) and provide + // appropriate migration guidance. + return []string{installer.ScopeGlobal}, nil + } +} + +// resolveScopeForUninstall resolves the scope for the uninstall command. +// 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") + } + + hasGlobal, hasProject, err := detectInstalledScopes(globalDir, projectDir) + if err != nil { + return "", err + } + + if projectFlag { + scopes, err := withExplicitScopeCheck(projectDir, installer.ScopeProject, "uninstall", projectDir, hasGlobal, hasProject) + if err != nil { + return "", err + } + return scopes[0], nil + } + if globalFlag { + // Always pass through to the installer layer, which handles legacy installs. + return installer.ScopeGlobal, nil + } + + // No flags: auto-detect. + 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") + } + scope, err := promptUninstallScopeSelection(ctx) + if err != nil { + return "", err + } + return scope, nil + + case hasGlobal: + return installer.ScopeGlobal, nil + + case hasProject: + return installer.ScopeProject, nil + + default: + // Fall through to global scope so the installer layer can detect + // legacy installs (skills on disk without .state.json) and provide + // appropriate migration guidance. + return installer.ScopeGlobal, nil + } +} + +// withExplicitScopeCheck validates that the explicitly requested scope has an installation. +// Returns a helpful error with CWD guidance for project scope and cross-scope hints. +// The verb parameter (e.g. "update", "uninstall") is used in cross-scope hint messages. +func withExplicitScopeCheck(dir, scope, verb, projectDir string, hasGlobal, hasProject bool) ([]string, error) { + state, err := installer.LoadState(dir) + if err != nil { + return nil, err + } + if state == nil { + return nil, scopeNotInstalledError(scope, verb, projectDir, hasGlobal, hasProject) + } + + return []string{scope}, nil +} + +// scopeNotInstalledError builds a detailed error for when the requested scope has no installation. +// Includes cross-scope hints when the other scope is installed. +// The verb parameter (e.g. "update", "uninstall") is used in cross-scope hint messages. +func scopeNotInstalledError(scope, verb, projectDir string, hasGlobal, hasProject bool) error { + var msg string + if scope == installer.ScopeProject { + expectedPath := filepath.ToSlash(projectDir) + msg = fmt.Sprintf( + "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 experimental aitools install --project'.\n\n"+ + "Expected location: %s/", expectedPath) + } else { + msg = "no globally-scoped skills installed. Run 'databricks experimental aitools install --global' to install" + } + + hint := crossScopeHint(scope, verb, hasGlobal, hasProject) + if hint != "" { + msg += "\n\n" + hint + } + + return errors.New(msg) +} + +// crossScopeHint returns a hint string if the opposite scope has an installation. +// 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) + } + if requestedScope == installer.ScopeGlobal && hasProject { + return fmt.Sprintf("Project-scoped skills are installed. Run without --global to %s those.", verb) + } + return "" +} + +func defaultPromptUpdateScopeSelection(ctx context.Context) ([]string, error) { + homeDir, err := env.UserHomeDir(ctx) + if err != nil { + return nil, err + } + globalPath := filepath.Join(homeDir, ".databricks", "aitools", "skills") + + cwd, err := os.Getwd() + if err != nil { + return nil, err + } + projectPath := filepath.Join(cwd, ".databricks", "aitools", "skills") + + globalLabel := "Global (" + globalPath + "/)" + projectLabel := "Project (" + projectPath + "/)" + bothLabel := "Both global and project" + + var scope string + err = huh.NewSelect[string](). + Title("Which installation should be updated?"). + Options( + huh.NewOption(globalLabel, installer.ScopeGlobal), + huh.NewOption(projectLabel, installer.ScopeProject), + huh.NewOption(bothLabel, scopeBoth), + ). + Value(&scope). + Run() + if err != nil { + return nil, err + } + + if scope == scopeBoth { + return []string{installer.ScopeGlobal, installer.ScopeProject}, nil + } + return []string{scope}, nil +} + +func defaultPromptUninstallScopeSelection(ctx context.Context) (string, error) { + homeDir, err := env.UserHomeDir(ctx) + if err != nil { + return "", err + } + globalPath := filepath.Join(homeDir, ".databricks", "aitools", "skills") + + cwd, err := os.Getwd() + if err != nil { + return "", err + } + projectPath := filepath.Join(cwd, ".databricks", "aitools", "skills") + + globalLabel := "Global (" + globalPath + "/)" + projectLabel := "Project (" + projectPath + "/)" + + var scope string + err = huh.NewSelect[string](). + Title("Which installation should be uninstalled?"). + Options( + huh.NewOption(globalLabel, installer.ScopeGlobal), + huh.NewOption(projectLabel, installer.ScopeProject), + ). + Value(&scope). + Run() + if err != nil { + return "", err + } + + return scope, nil +} diff --git a/experimental/aitools/cmd/scope_test.go b/experimental/aitools/cmd/scope_test.go new file mode 100644 index 0000000000..ecda25faad --- /dev/null +++ b/experimental/aitools/cmd/scope_test.go @@ -0,0 +1,571 @@ +package aitools + +import ( + "bufio" + "context" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/databricks/cli/libs/cmdio" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func setupScopeTest(t *testing.T) (globalDir, projectDir string) { + t.Helper() + homeDir := t.TempDir() + projectRoot := t.TempDir() + globalDir = filepath.Join(homeDir, ".databricks", "aitools", "skills") + projectDir = filepath.Join(projectRoot, ".databricks", "aitools", "skills") + return globalDir, projectDir +} + +func installGlobalState(t *testing.T, globalDir string) { + t.Helper() + require.NoError(t, os.MkdirAll(globalDir, 0o755)) + require.NoError(t, installer.SaveState(globalDir, &installer.InstallState{ + SchemaVersion: 1, + Release: "v0.1.0", + Skills: map[string]string{"test-skill": "1.0.0"}, + })) +} + +func installProjectState(t *testing.T, projectDir string) { + t.Helper() + require.NoError(t, os.MkdirAll(projectDir, 0o755)) + require.NoError(t, installer.SaveState(projectDir, &installer.InstallState{ + SchemaVersion: 1, + Release: "v0.1.0", + Skills: map[string]string{"test-skill": "1.0.0"}, + Scope: installer.ScopeProject, + })) +} + +func nonInteractiveCtx(t *testing.T) context.Context { + t.Helper() + return cmdio.MockDiscard(t.Context()) +} + +func interactiveCtx(t *testing.T) (context.Context, func()) { + t.Helper() + ctx, test := cmdio.SetupTest(t.Context(), cmdio.TestOptions{PromptSupported: true}) + + drain := func(r *bufio.Reader) { + buf := make([]byte, 4096) + for { + _, err := r.Read(buf) + if err != nil { + return + } + } + } + go drain(test.Stdout) + go drain(test.Stderr) + + return ctx, test.Done +} + +// --- detectInstalledScopes tests (table-driven) --- + +func TestDetectInstalledScopes(t *testing.T) { + tests := []struct { + name string + installGlob bool + installProj bool + wantGlobal bool + wantProject bool + }{ + {"Both", true, true, true, true}, + {"GlobalOnly", true, false, true, false}, + {"ProjectOnly", false, true, false, true}, + {"Neither", false, false, false, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + if tt.installGlob { + installGlobalState(t, globalDir) + } + if tt.installProj { + installProjectState(t, projectDir) + } + + hasGlobal, hasProject, err := detectInstalledScopes(globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, tt.wantGlobal, hasGlobal) + assert.Equal(t, tt.wantProject, hasProject) + }) + } +} + +// --- withExplicitScopeCheck tests (table-driven) --- + +func TestWithExplicitScopeCheck(t *testing.T) { + tests := []struct { + name string + scope string + installGlob bool + installProj bool + wantScopes []string + wantErr string + }{ + { + name: "ProjectPresent", + scope: installer.ScopeProject, + installProj: true, + wantScopes: []string{installer.ScopeProject}, + }, + { + name: "GlobalPresent", + scope: installer.ScopeGlobal, + installGlob: true, + wantScopes: []string{installer.ScopeGlobal}, + }, + { + name: "ProjectMissing", + scope: installer.ScopeProject, + wantErr: "no project-scoped skills found", + }, + { + name: "GlobalMissing", + scope: installer.ScopeGlobal, + wantErr: "no globally-scoped skills installed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + if tt.installGlob { + installGlobalState(t, globalDir) + } + if tt.installProj { + installProjectState(t, projectDir) + } + + hasGlobal, hasProject, err := detectInstalledScopes(globalDir, projectDir) + require.NoError(t, err) + + var dir string + if tt.scope == installer.ScopeProject { + dir = projectDir + } else { + dir = globalDir + } + + scopes, err := withExplicitScopeCheck(dir, tt.scope, "update", projectDir, hasGlobal, hasProject) + if tt.wantErr != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + } else { + require.NoError(t, err) + assert.Equal(t, tt.wantScopes, scopes) + } + }) + } +} + +// --- cross-scope hint tests --- + +func TestCrossScopeHint(t *testing.T) { + tests := []struct { + name string + scope string + verb string + hasGlobal bool + hasProj bool + wantHint string + }{ + { + name: "ProjectMissingHintsGlobalUpdate", + scope: installer.ScopeProject, + verb: "update", + hasGlobal: true, + wantHint: "without --project to update those", + }, + { + name: "ProjectMissingHintsGlobalUninstall", + scope: installer.ScopeProject, + verb: "uninstall", + hasGlobal: true, + wantHint: "without --project to uninstall those", + }, + { + name: "GlobalMissingHintsProject", + scope: installer.ScopeGlobal, + verb: "update", + hasProj: true, + wantHint: "without --global to update those", + }, + { + name: "NeitherInstalledNoHint", + scope: installer.ScopeProject, + verb: "update", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hint := crossScopeHint(tt.scope, tt.verb, tt.hasGlobal, tt.hasProj) + if tt.wantHint != "" { + assert.Contains(t, hint, tt.wantHint) + } else { + assert.Empty(t, hint) + } + }) + } +} + +// --- scopeNotInstalledError tests --- + +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(), "Expected location:") + assert.Contains(t, err.Error(), "/some/project/.databricks/aitools/skills/") +} + +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") +} + +// --- resolveScopeForUpdate tests --- + +func TestResolveScopeForUpdateBothFlagsBothInstalled(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + installProjectState(t, projectDir) + ctx := nonInteractiveCtx(t) + + scopes, err := resolveScopeForUpdate(ctx, true, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeGlobal, installer.ScopeProject}, scopes) +} + +func TestResolveScopeForUpdateBothFlagsOnlyGlobalInstalled(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + ctx := nonInteractiveCtx(t) + + scopes, err := resolveScopeForUpdate(ctx, true, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeGlobal}, scopes) +} + +func TestResolveScopeForUpdateBothFlagsOnlyProjectInstalled(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installProjectState(t, projectDir) + ctx := nonInteractiveCtx(t) + + // Only project is installed, so only project is returned. + scopes, err := resolveScopeForUpdate(ctx, true, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeProject}, scopes) +} + +func TestResolveScopeForUpdateBothFlagsNeitherInstalled(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + ctx := nonInteractiveCtx(t) + + // Neither installed: falls through to global for legacy detection. + scopes, err := resolveScopeForUpdate(ctx, true, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeGlobal}, scopes) +} + +func TestResolveScopeForUpdateProjectFlagWithState(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installProjectState(t, projectDir) + ctx := nonInteractiveCtx(t) + + scopes, err := resolveScopeForUpdate(ctx, true, false, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeProject}, scopes) +} + +func TestResolveScopeForUpdateGlobalFlagWithState(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + ctx := nonInteractiveCtx(t) + + scopes, err := resolveScopeForUpdate(ctx, false, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeGlobal}, scopes) +} + +func TestResolveScopeForUpdateProjectFlagNoInstall(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + ctx := nonInteractiveCtx(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(), "Expected location:") + assert.Contains(t, err.Error(), ".databricks/aitools/skills/") +} + +func TestResolveScopeForUpdateGlobalFlagNoInstall(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + ctx := nonInteractiveCtx(t) + + // Global flag always passes through to installer layer (for legacy install detection). + scopes, err := resolveScopeForUpdate(ctx, false, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeGlobal}, scopes) +} + +func TestResolveScopeForUpdateNoFlagsGlobalOnly(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + ctx := nonInteractiveCtx(t) + + scopes, err := resolveScopeForUpdate(ctx, false, false, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeGlobal}, scopes) +} + +func TestResolveScopeForUpdateNoFlagsProjectOnly(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installProjectState(t, projectDir) + ctx := nonInteractiveCtx(t) + + scopes, err := resolveScopeForUpdate(ctx, false, false, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeProject}, scopes) +} + +func TestResolveScopeForUpdateNoFlagsBothNonInteractive(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + installProjectState(t, projectDir) + ctx := nonInteractiveCtx(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") +} + +func TestResolveScopeForUpdateNoFlagsBothInteractive(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + installProjectState(t, projectDir) + + orig := promptUpdateScopeSelection + t.Cleanup(func() { promptUpdateScopeSelection = orig }) + promptCalled := false + promptUpdateScopeSelection = func(_ context.Context) ([]string, error) { + promptCalled = true + return []string{installer.ScopeGlobal}, nil + } + + ctx, done := interactiveCtx(t) + defer done() + + scopes, err := resolveScopeForUpdate(ctx, false, false, globalDir, projectDir) + require.NoError(t, err) + assert.True(t, promptCalled) + assert.Equal(t, []string{installer.ScopeGlobal}, scopes) +} + +func TestResolveScopeForUpdateNoFlagsNeitherInstalled(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + ctx := nonInteractiveCtx(t) + + // Falls through to global so the installer layer can handle legacy installs. + scopes, err := resolveScopeForUpdate(ctx, false, false, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeGlobal}, scopes) +} + +// --- resolveScopeForUninstall tests --- + +func TestResolveScopeForUninstallBothFlagsError(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + ctx := nonInteractiveCtx(t) + + _, err := resolveScopeForUninstall(ctx, true, true, globalDir, projectDir) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot uninstall both scopes at once") +} + +func TestResolveScopeForUninstallProjectFlagWithState(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installProjectState(t, projectDir) + ctx := nonInteractiveCtx(t) + + scope, err := resolveScopeForUninstall(ctx, true, false, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, installer.ScopeProject, scope) +} + +func TestResolveScopeForUninstallGlobalFlagWithState(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + ctx := nonInteractiveCtx(t) + + scope, err := resolveScopeForUninstall(ctx, false, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, installer.ScopeGlobal, scope) +} + +func TestResolveScopeForUninstallProjectFlagNoInstall(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + ctx := nonInteractiveCtx(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(), "Expected location:") +} + +func TestResolveScopeForUninstallGlobalFlagNoInstall(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + ctx := nonInteractiveCtx(t) + + // Global flag always passes through to installer layer (for legacy install detection). + scope, err := resolveScopeForUninstall(ctx, false, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, installer.ScopeGlobal, scope) +} + +func TestResolveScopeForUninstallNoFlagsGlobalOnly(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + ctx := nonInteractiveCtx(t) + + scope, err := resolveScopeForUninstall(ctx, false, false, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, installer.ScopeGlobal, scope) +} + +func TestResolveScopeForUninstallNoFlagsProjectOnly(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installProjectState(t, projectDir) + ctx := nonInteractiveCtx(t) + + scope, err := resolveScopeForUninstall(ctx, false, false, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, installer.ScopeProject, scope) +} + +func TestResolveScopeForUninstallNoFlagsBothNonInteractive(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + installProjectState(t, projectDir) + ctx := nonInteractiveCtx(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") +} + +func TestResolveScopeForUninstallNoFlagsBothInteractive(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + installProjectState(t, projectDir) + + orig := promptUninstallScopeSelection + t.Cleanup(func() { promptUninstallScopeSelection = orig }) + promptCalled := false + promptUninstallScopeSelection = func(_ context.Context) (string, error) { + promptCalled = true + return installer.ScopeProject, nil + } + + ctx, done := interactiveCtx(t) + defer done() + + scope, err := resolveScopeForUninstall(ctx, false, false, globalDir, projectDir) + require.NoError(t, err) + assert.True(t, promptCalled) + assert.Equal(t, installer.ScopeProject, scope) +} + +func TestResolveScopeForUninstallNoFlagsNeitherInstalled(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + ctx := nonInteractiveCtx(t) + + // Falls through to global so the installer layer can handle legacy installs. + scope, err := resolveScopeForUninstall(ctx, false, false, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, installer.ScopeGlobal, scope) +} + +// --- legacy global install passthrough tests --- + +func TestResolveScopeForUpdateGlobalFlagLegacyInstall(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + // Create skill directories on disk but no .state.json (legacy install). + legacyDir := filepath.Join(globalDir, "some-skill") + require.NoError(t, os.MkdirAll(legacyDir, 0o755)) + ctx := nonInteractiveCtx(t) + + // Global flag should pass through to installer layer, not block on missing state. + scopes, err := resolveScopeForUpdate(ctx, false, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeGlobal}, scopes) +} + +func TestResolveScopeForUninstallGlobalFlagLegacyInstall(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + // Create skill directories on disk but no .state.json (legacy install). + legacyDir := filepath.Join(globalDir, "some-skill") + require.NoError(t, os.MkdirAll(legacyDir, 0o755)) + ctx := nonInteractiveCtx(t) + + // Global flag should pass through to installer layer, not block on missing state. + scope, err := resolveScopeForUninstall(ctx, false, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, installer.ScopeGlobal, scope) +} + +func TestResolveScopeForUpdateBothFlagsLegacyGlobal(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + // Global has a legacy install (dirs on disk, no state), project has state. + legacyDir := filepath.Join(globalDir, "some-skill") + require.NoError(t, os.MkdirAll(legacyDir, 0o755)) + installProjectState(t, projectDir) + ctx := nonInteractiveCtx(t) + + // Legacy global has no state, so only project (which has state) is included. + scopes, err := resolveScopeForUpdate(ctx, true, true, globalDir, projectDir) + require.NoError(t, err) + assert.Equal(t, []string{installer.ScopeProject}, scopes) +} + +// --- uninstall cross-scope hint verb tests --- + +func TestResolveScopeForUninstallProjectFlagHintsUninstall(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + ctx := nonInteractiveCtx(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") +} + +func TestResolveScopeForUpdateProjectFlagHintsUpdate(t *testing.T) { + globalDir, projectDir := setupScopeTest(t) + installGlobalState(t, globalDir) + ctx := nonInteractiveCtx(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") +} diff --git a/experimental/aitools/cmd/uninstall.go b/experimental/aitools/cmd/uninstall.go index 3554e85faf..3eda84cfbc 100644 --- a/experimental/aitools/cmd/uninstall.go +++ b/experimental/aitools/cmd/uninstall.go @@ -17,7 +17,18 @@ func newUninstallCmd() *cobra.Command { By default, removes all skills. Use --skills to remove specific skills only.`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - scope, err := resolveScope(projectFlag, globalFlag) + ctx := cmd.Context() + + globalDir, err := installer.GlobalSkillsDir(ctx) + if err != nil { + return err + } + projectDir, err := installer.ProjectSkillsDir(ctx) + if err != nil { + return err + } + + scope, err := resolveScopeForUninstall(ctx, projectFlag, globalFlag, globalDir, projectDir) if err != nil { return err } @@ -26,12 +37,12 @@ By default, removes all skills. Use --skills to remove specific skills only.`, Scope: scope, } opts.Skills = splitAndTrim(skillsFlag) - return installer.UninstallSkillsOpts(cmd.Context(), opts) + return installer.UninstallSkillsOpts(ctx, opts) }, } cmd.Flags().StringVar(&skillsFlag, "skills", "", "Specific skills to uninstall (comma-separated)") cmd.Flags().BoolVar(&projectFlag, "project", false, "Uninstall project-scoped skills") - cmd.Flags().BoolVar(&globalFlag, "global", false, "Uninstall globally-scoped skills (default)") + cmd.Flags().BoolVar(&globalFlag, "global", false, "Uninstall globally-scoped skills") return cmd } diff --git a/experimental/aitools/cmd/update.go b/experimental/aitools/cmd/update.go index fadea42488..c5072d1fb1 100644 --- a/experimental/aitools/cmd/update.go +++ b/experimental/aitools/cmd/update.go @@ -1,6 +1,8 @@ package aitools import ( + "fmt" + "github.com/databricks/cli/experimental/aitools/lib/agents" "github.com/databricks/cli/experimental/aitools/lib/installer" "github.com/databricks/cli/libs/cmdio" @@ -24,28 +26,44 @@ preview what would change without downloading.`, RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - scope, err := resolveScope(projectFlag, globalFlag) + globalDir, err := installer.GlobalSkillsDir(ctx) if err != nil { return err } - - installed := agents.DetectInstalled(ctx) - src := &installer.GitHubManifestSource{} - - opts := installer.UpdateOptions{ - Check: check, - Force: force, - NoNew: noNew, - Scope: scope, + projectDir, err := installer.ProjectSkillsDir(ctx) + if err != nil { + return err } - opts.Skills = splitAndTrim(skillsFlag) - result, err := installer.UpdateSkills(ctx, src, installed, opts) + scopes, err := resolveScopeForUpdate(ctx, projectFlag, globalFlag, globalDir, projectDir) if err != nil { return err } - if result != nil && (len(result.Updated) > 0 || len(result.Added) > 0) { - cmdio.LogString(ctx, installer.FormatUpdateResult(result, check)) + + installed := agents.DetectInstalled(ctx) + src := &installer.GitHubManifestSource{} + skills := splitAndTrim(skillsFlag) + + for _, scope := range scopes { + if len(scopes) > 1 { + cmdio.LogString(ctx, fmt.Sprintf("Updating %s skills...", scope)) + } + + opts := installer.UpdateOptions{ + Check: check, + Force: force, + NoNew: noNew, + Scope: scope, + } + opts.Skills = skills + + result, err := installer.UpdateSkills(ctx, src, installed, opts) + if err != nil { + return err + } + if result != nil && (len(result.Updated) > 0 || len(result.Added) > 0) { + cmdio.LogString(ctx, installer.FormatUpdateResult(result, check)) + } } return nil }, @@ -56,6 +74,6 @@ preview what would change without downloading.`, 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().BoolVar(&projectFlag, "project", false, "Update project-scoped skills") - cmd.Flags().BoolVar(&globalFlag, "global", false, "Update globally-scoped skills (default)") + cmd.Flags().BoolVar(&globalFlag, "global", false, "Update globally-scoped skills") return cmd }