aitools: add --scope flag, deprecate --project/--global by jamesbroadhead · Pull Request #5234 · databricks/cli

jamesbroadhead · 2026-05-11T14:52:37Z

Summary

Split out from #4917. While that PR keeps responsibility for moving the aitools skills-management surface out of experimental/, this PR makes the user-facing interface changes that should land at the same moment:

New --scope=project|global flag on install/update/uninstall/list, with --scope=both accepted by update and list.
--project and --global are marked deprecated via cobra's Deprecated property: hidden from --help, emit a stderr deprecation warning when used, continue to function so existing scripts don't break. They're slated for removal in a later release.
--scope combined with --project/--global is rejected up front with an actionable error.
install's --help now documents the non-interactive --agents auto-detect contract so callers know what gets picked.

Stacked on #4917. Base will rebase to main once that lands. Splitting because (a) #4917 is otherwise a pure file move and reviewers asked to keep it that way, and (b) the interface change has its own product question (boolean pair vs. enum) worth landing as a discrete unit.

Why land this with the rename

aitools is being declared a stable top-level surface in #4917. This is the cheapest moment to fix the two-boolean shape before external scripts depend on it. An enum is also better for agent-driven invocations than two booleans with implicit precedence: --scope=project|global|both is one flag with valid values, not two flags with order-dependent semantics.

Surface

databricks aitools install   --scope=project|global             (--scope=both rejected)
databricks aitools uninstall --scope=project|global             (--scope=both rejected)
databricks aitools update    --scope=project|global|both
databricks aitools list      --scope=project|global|both        (default: both)

databricks aitools install --project    # warns: use --scope=project
databricks aitools install --global     # warns: use --scope=global

Test plan

databricks aitools install --scope=project and --scope=global go to the right destination
databricks aitools install --scope=both errors with a clear message
databricks aitools install --project still works and prints the deprecation warning to stderr
databricks aitools install --scope=global --project errors with the conflict message
databricks aitools list --scope=both shows both scopes (equivalent to no flag)
databricks aitools install --help no longer shows --project/--global; --scope is documented; --agents auto-detect behavior is described
Unit: TestParseScopeFlag (table-driven on the translation), TestInstallScopeFlag, TestListScopeFlag — all green

This pull request was AI-assisted by Isaac.

jamesbroadhead · 2026-05-18T21:43:49Z

👋 @simonfaltum — Claude here on James's behalf.

#4917 merged earlier today, so this PR is now stacked on main and no longer needs to wait. I've rebased the branch onto current main and force-pushed; the diff is now the actual scope-flag delta (9 files, +219/-13) without the merge noise from the old base.

Ready for a re-review whenever you've got a moment.

(comment posted by Claude)

jamesbroadhead · 2026-05-19T08:41:40Z

👋 @simonfaltum — Claude here on James's behalf.

All three of your should-fix items are addressed in 24d5351fa:

list.go silent semantics regression — moved the validation into parseScopeFlag itself so combining the two deprecated booleans is always an error, regardless of allowBoth. Restored the original message: cannot use --global and --project together. Updated the existing TestListScopeFlag/legacy_both_flags_shows_both case to expect the error (renamed accordingly).
allowBoth=false error message — parseScopeFlag now branches: with allowBoth=true the message says "must be one of project, global, both"; with allowBoth=false (install, uninstall), the message says "must be one of project, global". Added a table case + an explicit NotContains "both" assertion in TestParseScopeFlag since the substring check on the table would have shared a prefix with the allowBoth variant.
Test gap on update/uninstall — added cmd/aitools/update_test.go (TestUpdateScopeFlag) and cmd/aitools/uninstall_test.go (TestUninstallScopeFlag), each table-driven covering: a success case that exercises the new wiring (--scope=global for both, plus --scope=both for update which falls through to global without state), --scope=all invalid value, the --scope=X --project conflict, and --project --global together. To support those without hitting the network, introduced updateSkillsFn and uninstallSkillsFn package-level test-injection vars mirroring the installSkillsForAgentsFn pattern already in install.go. --scope=project success paths stay covered via TestParseScopeFlag and the existing TestResolveScopeForUpdate/UninstallProjectFlagWithState (those need installed state, which is fiddly to set up in a command-level test).

go test ./cmd/aitools passes locally.

(comment posted by Claude)

Teaches list to render as a structured {release, skills[...], summary{}} document when --output json is passed. Text rendering is unchanged. Stacked on jb/aitools-interface (#5234). Original branch was rebased onto current main + that PR's tip; layout drift from #4917's pre-merge shape was reconciled (cmd/aitools/* paths, unexported listSkillsFn, 3-value installer.GetSkillsRef signature). Co-authored-by: Isaac

simonfaltum

Fresh re-review after the fixup. The original findings look addressed, but I found one new update regression plus a couple of follow-ups around the deprecated flag UX.

simonfaltum · 2026-05-19T10:06:44Z

+	if scopeFlag == "" {
+		// Preserve the pre-refactor behavior: combining the two deprecated flags
+		// is always wrong, regardless of allowBoth. Users who want both scopes
+		// should use --scope=both (where supported).
+		if projectFlag && globalFlag {
+			return false, false, errors.New("cannot use --global and --project together")
+		}
+		return projectFlag, globalFlag, nil


Should-fix: this creates a new regression for aitools update --project --global. On main, resolveScopeForUpdate(ctx, true, true, ...) is the supported "update both scopes" path, and the existing resolver tests still cover that behavior. With this shared rejection in front of resolveScopeForUpdate, existing update scripts using the two deprecated flags now fail, even though the PR says the deprecated flags continue to function. Can we preserve legacy both-flag behavior for update until those flags are removed, while still rejecting it for install/list/uninstall?

simonfaltum · 2026-05-19T10:06:44Z

+// 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
+}


Since this hides --project and --global, the remaining user-facing guidance should stop recommending those hidden deprecated flags. The auto-detect and not-installed errors below still say things like use --global, --project, or both flags, install --project, install --global, and Run without --project. Those should point users at --scope=project, --scope=global, or --scope=both instead, so the error messages match the new public surface.

simonfaltum · 2026-05-19T10:06:44Z

 			}
-			// For list: no flag = show both scopes (empty string).
+
+			// list: empty scope = show both. Both flags set is equivalent.


Nit: this comment is stale after the fixup. Both deprecated flags are now rejected by parseScopeFlag; the "both" behavior is only the empty/default scope or explicit --scope=both.

- Move the legacy `--project --global` rejection out of parseScopeFlag and into list.go's RunE only. update preserves the supported "update both scopes" path through resolveScopeForUpdate; install relies on its existing resolveScope check (same error); uninstall errors downstream via resolveScopeForUninstall. - Rewrite user-facing error/hint messages in scope.go to point at the new public surface (--scope=project / --scope=global / --scope=both) instead of the now-hidden --project / --global flags. - Fix the stale comment in list.go ("Both flags set is equivalent" was no longer accurate after the fixup). - Update affected tests: revert TestParseScopeFlag's legacy-both case to passthrough; rewrite TestCrossScopeHint and the TestScopeNotInstalledError* asserts to the new flag spellings; change TestUpdateScopeFlag's legacy-both case to a non-error (falls through to global without state); update TestUninstallScopeFlag's expected error to come from resolveScopeForUninstall. Co-authored-by: Isaac

jamesbroadhead · 2026-05-19T20:42:47Z

👋 @simonfaltum — Claude here on James's behalf.

R2 fixup in d5409ebb1:

update --project --global regression — moved the legacy-both rejection out of parseScopeFlag and into list.go's RunE only. update keeps the supported "update both scopes" path through resolveScopeForUpdate (the existing TestResolveScopeForUpdateBothFlagsBothInstalled covers it). install relies on its existing resolveScope check (same error message); uninstall errors downstream via resolveScopeForUninstall ("cannot uninstall both scopes at once").
Hidden-flag mentions in error messages — rewrote all the user-facing strings in scope.go to point at --scope=...:
- "skills are installed in both global and project scopes; use --scope=global, --scope=project, or --scope=both to specify which to update" (auto-detect, non-interactive)
- "skills are installed in both global and project scopes; use --scope=global or --scope=project to specify which to uninstall"
- "cannot uninstall both scopes at once; run uninstall separately with --scope=global and --scope=project"
- scopeNotInstalledError: "Run 'databricks aitools install --scope=project'" / "--scope=global"
- crossScopeHint: "Run with --scope=global to %s those" / "Run with --scope=project to %s those"
Stale comment in list.go — fixed; the surrounding logic comment now reads "list: empty scope = show both. --scope=both also lands here." (both-bools-set is now an early error a few lines above).

Tests updated to match: TestParseScopeFlag legacy-both case reverts to passthrough; TestCrossScopeHint + TestScopeNotInstalledError* adjusted to the new flag spellings; TestUpdateScopeFlag's legacy-both case is now a non-error (falls through to global without state); TestUninstallScopeFlag expects the downstream resolveScopeForUninstall error.

go test ./cmd/aitools/... ./libs/aitools/... passes locally.

(comment posted by Claude)

## Summary Promotes the **aitools skills-management surface** out of `experimental/` so the stable half lives at `databricks aitools …` and slots in next to the other top-level command groups. The matching **interface changes** (`--scope` enum, `--project`/`--global` deprecation, `--agents` auto-detect doc) live in a stacked follow-up: **#5234**. This is mostly a move, but it is not move-only — see [Non-move changes](#non-move-changes) below. - Source files for `install`, `update`, `uninstall`, `list`, `version` (and the agents/installer libs they depend on) physically move from `experimental/aitools/` to `cmd/aitools/` + `libs/aitools/`, matching the existing `cmd/apps/` + `libs/apps/` layout. OWNERS, Taskfile, and the pr-checklist skill are updated to match. - The top-level command is registered at `databricks aitools …`. - **Keeps the `tools` subtree under `experimental/aitools/`** — `query`, `discover-schema`, `get-default-warehouse`, `statement …` — because `tools.go` still says "There are no stability guarantees for these tools". - The old paths under `databricks experimental aitools install/update/uninstall/list/version` and `databricks experimental aitools skills install/list` keep working as **deprecated backward-compat aliases** that print a notice pointing at the new path (via cobra's `Deprecated` field). The aitools skills-management surface is feature-complete after the 5-PR series (#4810–#4814) that added state tracking, lifecycle commands, and project scope support. The `tools` subtree is functionally useful but its shape is still in flux, so promoting only the stable half. ## Non-move changes In addition to the file moves, this PR: - Removes the redundant `aitools/README.md` (apps/pipelines don't carry one); the same info lives in the command's Long description. - Rewrites the Long description on the new top-level `databricks aitools` command. - Adds a deprecation notice to every legacy alias under `databricks experimental aitools` (Lennart's review ask — they used to forward silently). - Refactors how the legacy `experimental aitools skills install [name]` wrapper is wired: the wrapper now lives in `cmd/aitools/legacy_skills.go` alongside the install code it wraps, and the previously-exported test-injection vars (`InstallSkillsForAgentsFn`, `ListSkillsFn`, `PromptAgentSelection`) are back to unexported. The two now-empty experimental files (`experimental/aitools/cmd/skills.go` and `skills_test.go`) are deleted. ## What's not in this PR These are deliberately separated and reviewed independently: - **#5234** — `--scope=project|global|both` flag, deprecation of `--project`/`--global` via `cobra.Deprecated`, `--agents` auto-detect help text. - **#5233** (draft) — `--output json` on `databricks aitools list`. ## Command shape after this PR ``` # Stable, top-level databricks aitools install # use --skills <name>[,<name>...] for specific skills databricks aitools update databricks aitools uninstall databricks aitools list databricks aitools version # Backward-compat aliases (print deprecation notice; point at the new paths) databricks experimental aitools install/update/uninstall/list/version databricks experimental aitools skills {list,install} # Experimental, unchanged path databricks experimental aitools tools query databricks experimental aitools tools discover-schema databricks experimental aitools tools get-default-warehouse databricks experimental aitools tools statement {submit,get,status,cancel} ``` ## Test plan - [x] `databricks aitools --help` shows install/update/uninstall/list/version (no `tools`) - [x] `databricks --help` lists `aitools` in the output - [x] `databricks experimental aitools install` prints a deprecation notice and still forwards - [x] `databricks experimental aitools tools query …` runs as before - [x] `databricks experimental aitools tools --help` lists query/discover-schema/get-default-warehouse/statement - [x] Existing aitools tests pass; the legacy-wrapper tests moved with the wrapper to `cmd/aitools/legacy_skills_test.go` This pull request was AI-assisted by Isaac. --------- Co-authored-by: simon <simon.faltum@databricks.com> Co-authored-by: simon <4305831+simonfaltum@users.noreply.github.com>

simonfaltum

Looks good. The earlier findings are addressed, and the changelog now preserves the unrelated main entry while documenting the new scope flag.

Teaches list to render as a structured {release, skills[...], summary{}} document when --output json is passed. Text rendering is unchanged. Stacked-on-#5234 rebased onto main now that #5234 has merged. The branch state was carrying stale rehashes of the scope-flag work; squashed onto current main to keep only the JSON-output delta. Co-authored-by: Isaac

#5234 added a deprecation message when --global is passed; regenerate the affected aitools install goldens so the acceptance tests pass after the merge from main. Co-authored-by: Isaac

jamesbroadhead requested a review from simonfaltum May 11, 2026 14:52

jamesbroadhead temporarily deployed to test-trigger-is May 11, 2026 14:53 — with GitHub Actions Inactive

jamesbroadhead mentioned this pull request May 11, 2026

Promote aitools skills-management commands to top-level #4917

Merged

6 tasks

jamesbroadhead had a problem deploying to test-trigger-is May 11, 2026 15:41 — with GitHub Actions Failure

jamesbroadhead temporarily deployed to test-trigger-is May 11, 2026 15:46 — with GitHub Actions Inactive

jamesbroadhead force-pushed the jb/aitools-interface branch from 526cd9c to a7e3d03 Compare May 12, 2026 20:13

jamesbroadhead temporarily deployed to test-trigger-is May 12, 2026 20:14 — with GitHub Actions Inactive

jamesbroadhead force-pushed the jb/aitools-interface branch from a7e3d03 to 37cf784 Compare May 13, 2026 14:32

jamesbroadhead temporarily deployed to test-trigger-is May 13, 2026 14:33 — with GitHub Actions Inactive

simonfaltum temporarily deployed to test-trigger-is May 18, 2026 17:39 — with GitHub Actions Inactive

simonfaltum temporarily deployed to test-trigger-is May 18, 2026 17:40 — with GitHub Actions Inactive

simonfaltum temporarily deployed to test-trigger-is May 18, 2026 18:01 — with GitHub Actions Inactive

simonfaltum temporarily deployed to test-trigger-is May 18, 2026 18:31 — with GitHub Actions Inactive

Base automatically changed from jbroadhead/aitools-public to main May 18, 2026 19:26

simonfaltum temporarily deployed to test-trigger-is May 18, 2026 19:31 — with GitHub Actions Inactive

simonfaltum temporarily deployed to test-trigger-is May 18, 2026 20:33 — with GitHub Actions Inactive

jamesbroadhead force-pushed the jb/aitools-interface branch from 8f0319a to 7666312 Compare May 18, 2026 21:43

jamesbroadhead temporarily deployed to test-trigger-is May 18, 2026 21:44 — with GitHub Actions Inactive

jamesbroadhead temporarily deployed to test-trigger-is May 19, 2026 08:41 — with GitHub Actions Inactive

simonfaltum reviewed May 19, 2026

View reviewed changes

Merge branch 'main' into jb/aitools-interface

7427834

simonfaltum temporarily deployed to test-trigger-is May 20, 2026 07:37 — with GitHub Actions Inactive

Update NEXT_CHANGELOG.md

f80f084

simonfaltum temporarily deployed to test-trigger-is May 20, 2026 07:37 — with GitHub Actions Inactive

Merge branch 'main' into jb/aitools-interface

e921bfb

simonfaltum temporarily deployed to test-trigger-is May 20, 2026 13:24 — with GitHub Actions Inactive

Merge branch 'main' into jb/aitools-interface

771d066

simonfaltum temporarily deployed to test-trigger-is May 20, 2026 14:26 — with GitHub Actions Inactive

Fix aitools changelog entry

84e2f2a

simonfaltum temporarily deployed to test-trigger-is May 20, 2026 14:32 — with GitHub Actions Inactive

simonfaltum approved these changes May 20, 2026

View reviewed changes

jamesbroadhead enabled auto-merge May 20, 2026 14:34

simonfaltum disabled auto-merge May 20, 2026 15:29

simonfaltum merged commit 2cefbcc into main May 20, 2026
25 of 26 checks passed

simonfaltum deleted the jb/aitools-interface branch May 20, 2026 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aitools: add --scope flag, deprecate --project/--global#5234

aitools: add --scope flag, deprecate --project/--global#5234
simonfaltum merged 8 commits into
mainfrom
jb/aitools-interface

jamesbroadhead commented May 11, 2026

Uh oh!

jamesbroadhead commented May 18, 2026

Uh oh!

jamesbroadhead commented May 19, 2026

Uh oh!

simonfaltum left a comment

Uh oh!

simonfaltum May 19, 2026

Uh oh!

simonfaltum May 19, 2026

Uh oh!

simonfaltum May 19, 2026

Uh oh!

jamesbroadhead commented May 19, 2026

Uh oh!

simonfaltum left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jamesbroadhead commented May 11, 2026

Summary

Why land this with the rename

Surface

Test plan

Uh oh!

jamesbroadhead commented May 18, 2026

Uh oh!

jamesbroadhead commented May 19, 2026

Uh oh!

simonfaltum left a comment

Choose a reason for hiding this comment

Uh oh!

simonfaltum May 19, 2026

Choose a reason for hiding this comment

Uh oh!

simonfaltum May 19, 2026

Choose a reason for hiding this comment

Uh oh!

simonfaltum May 19, 2026

Choose a reason for hiding this comment

Uh oh!

jamesbroadhead commented May 19, 2026

Uh oh!

simonfaltum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants