Commit 209fa87
aitools: parse experimental_skills manifest section (#5243)
## Summary
The skills manifest in `databricks/databricks-agent-skills` is gaining
experimental skills sourced from a new `experimental/` directory in the
repo (see paired [d-a-s PR
#73](databricks/databricks-agent-skills#73),
which imports the ai-dev-kit skill catalog into `experimental/`).
This wires the parsing through the aitools installer:
- `Manifest.Skills` is a **single map** holding both stable and
experimental entries; the per-skill `repo_dir` field ("skills" or
"experimental") is the source of truth for whether a skill is
experimental. `SkillMeta.IsExperimental()` derives state from `RepoDir`.
- Experimental skills get a `-experimental` suffix on their install-side
key during `normalizeManifest`; `SourceName` preserves the unsuffixed
name for fetch URLs.
- The existing `--experimental` flag (already wired in `cmd/skills.go`)
now has experimental skills to install; without it, `resolveSkills`
filters them out as before.
## UX
```
# default — only stable skills
databricks experimental aitools skills install
# all experimental skills, plus stable
databricks experimental aitools skills install --experimental
# one experimental skill by name (--experimental still required by resolveSkills)
databricks experimental aitools skills install databricks-iceberg-experimental --experimental
```
## TODOs / caveats for iteration
1. ~~**`DATABRICKS_SKILLS_REF` pin.**~~ **Partially resolved.** The
default ref is still the latest stable release tag (sourced from
`experimental/aitools/lib/installer/SKILLS_VERSION`); experimental
entries won't exist there until d-a-s cuts a release with [PR
#73](databricks/databricks-agent-skills#73)
merged. The default ref bump is a follow-up automated by the
SKILLS_VERSION file. **UX fix shipped in this PR**: if `--experimental`
is passed but the manifest at the resolved ref exposes no experimental
skills, a warning is logged pointing users at
`DATABRICKS_SKILLS_REF=main`.
2. ~~**Collision handling is naive.**~~ **Resolved.** Every experimental
skill gets a `-experimental` suffix on its install-side key during
`normalizeManifest`. The manifest key + install dir both carry the
suffix; the `SourceName` field on `SkillMeta` preserves the upstream
repo dir name for fetch URLs. Users see at a glance which installed
skills are experimental.
Also handled: **experimental↔stable transitions**. If a skill flips its
experimental status upstream (the same logical skill changes manifest
key), `install` removes the stale variant on disk + state before
installing the new one, and `uninstall` accepts either variant name (and
removes both if both are present). Helper: `alternateVariantKey()`.
Covered by tests `TestInstallReplacesAlternateVariant`,
`TestUninstallByEitherVariantRemovesBoth`,
`TestUninstallByAlternateNameWhenOnlyOneVariantInstalled`.
3. ~~**`list` UX.**~~ **Resolved.** `aitools skills list` shows
experimental skills with an `[experimental]` tag in the NAME column
(driven by `meta.IsExperimental()`). Combined with the TODO #2
resolution (`-experimental` suffix in the manifest key), every
experimental row reads e.g. `databricks-iceberg-experimental
[experimental]` — slightly redundant but a clear visual anchor.
Hide-by-default was considered but rejected: users running `list` are
usually looking for what's available, and silently omitting experimental
skills makes them un-discoverable.
4. ~~**State tracking.**~~ **Resolved — kept additive semantics.**
`InstallState.IncludeExperimental` records what was last requested but
is not used to drive retroactive removal. Running `install` without
`--experimental` leaves previously-installed experimental skills in
place. Rationale: (a) users running `install` are typically
adding/updating, not declaring set membership; (b) silently uninstalling
things the user previously asked for is surprising; (c) the transition
cleanup shipped under TODO #2 handles the actual drift case (skill's
experimental status flipping upstream). Removal is what `uninstall` is
for.
5. ~~**No acceptance test yet.**~~ **Resolved.** Added acceptance tests
under `acceptance/experimental/aitools/skills/install*/` covering the
install flow against a mocked manifest server:
- Stable-only install (no flag) → 1 skill installed
- `--experimental` install adds the experimental skill (with
`-experimental` suffix per the install-path model) → 2 skills total
- Re-running `--experimental` is idempotent
- Specific-skill install (`install --skills <name>`) for both stable and
experimental
- `--experimental` against a manifest with no experimental entries logs
a nudge
To make these reachable, exposed a new env-var override
`DATABRICKS_SKILLS_BASE_URL` that overrides the hard-coded
`raw.githubusercontent.com` base URL used by
`GitHubManifestSource.FetchManifest` and `fetchSkillFile`. Defaults to
the canonical URL when unset, so no production behavior change. Updated
`Taskfile.yml`'s `test-exp-aitools` task to include
`acceptance/experimental/aitools/**`.
Variants left as follow-up acceptance tests (the structure is now in
place):
- Variant transition cleanup (stable → experimental, experimental →
stable)
- Uninstall flow (with both variants installed)
6. ~~**`--experimental` flag scope.**~~ **Resolved — kept current
scope.** Each command has internally consistent behavior:
- `install --experimental` → explicit opt-in (required to install
experimental skills).
- `update` → state-driven (honors `InstallState.IncludeExperimental`
from the last `install`). If you opted in once, future updates refresh
experimentals; otherwise they're skipped.
- `list` → shows all skills with an `[experimental]` tag (no filtering —
discovery first, opt-in to install).
Adding `--experimental` / `--no-experimental` to `update` for one-off
overrides was considered but rejected: the natural workflow is to re-run
`install --experimental` (or just `install`), which already sets the
desired state. Follow-up if real users hit a use case for the override.
7. ~~**Manifest shape.**~~ **Resolved.** Replaced the original two-map
design (`skills` + `experimental_skills` + a per-skill `experimental`
bool) with a single `skills` map where each entry's `repo_dir`
(`"skills"` or `"experimental"`) is the source of truth. The cli derives
experimental state from `RepoDir` via `SkillMeta.IsExperimental()`.
Collisions between stable and experimental skills with the same repo dir
name must be resolved upstream in d-a-s (which they already are — d-a-s
PR #73's TODO #1a merged the only known collision into stable). The
d-a-s manifest generator should be updated to emit `repo_dir` per skill;
until then `normalizeManifest` defaults a missing `RepoDir` to
`"skills"` so older manifests still parse.
## Test plan
- [x] `go build ./...` passes.
- [x] `go test ./experimental/aitools/...` passes (`source_test.go`
covers the normalize/IsExperimental cases).
- [x] `go test ./acceptance -run TestAccept/experimental/aitools` passes
(a pre-existing flake intermittently surfaces an `lstat` warning during
copyDir, ~10% of multi-test runs; unrelated to this refactor).
- [ ] Run `./task lint` and `./task fmt` before merge.
- [ ] Manual: against a d-a-s ref containing experimental entries with
`repo_dir`, verify the four UX cases above behave correctly.
This pull request and its description were written by Claude.
---------
Co-authored-by: simon <4305831+simonfaltum@users.noreply.github.com>
Co-authored-by: simon <simon.faltum@databricks.com>1 parent d7b6edb commit 209fa87
23 files changed
Lines changed: 627 additions & 61 deletions
File tree
- acceptance/experimental/aitools/skills
- install-experimental-empty
- install-specific
- install
- cmd/aitools
- libs/aitools/installer
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
615 | 615 | | |
616 | 616 | | |
617 | 617 | | |
| 618 | + | |
618 | 619 | | |
619 | 620 | | |
620 | 621 | | |
| |||
628 | 629 | | |
629 | 630 | | |
630 | 631 | | |
631 | | - | |
| 632 | + | |
632 | 633 | | |
633 | 634 | | |
634 | 635 | | |
| |||
Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 9 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
Lines changed: 5 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
Lines changed: 31 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 26 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
Lines changed: 11 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
Lines changed: 50 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
0 commit comments