diff --git a/.agents/skills/bump-cli-compat/SKILL.md b/.agents/skills/bump-cli-compat/SKILL.md index a0612a2ba0e..454c93e14a9 100644 --- a/.agents/skills/bump-cli-compat/SKILL.md +++ b/.agents/skills/bump-cli-compat/SKILL.md @@ -64,7 +64,7 @@ If any tag doesn't exist, report the error and stop. ### Step 3: Read current manifest -Read `internal/build/cli-compat.json`. Note the current versions and the list of versioned entries. +Read `internal/build/cli-compat.json` and `AgentSkillsFallbackVersion` in `internal/build/clicompat.go`. Note the current versions and the list of versioned entries. The newest CLI range uses `"skills": "latest"`; the compiled constant supplies the tested fallback. Keep older released ranges pinned. Routine skills releases need no manifest bump. CLIs before `1.6.0` reject `"latest"` and use a valid cached or embedded manifest without returning a parse error; losing remote AppKit and skills compatibility updates for these CLIs is accepted. ### Step 4: Determine update type @@ -81,24 +81,24 @@ Ask the user: ### Step 4a: No breaking changes (update in-place) -Update the **highest versioned entry** to the new appkit and skills versions. Do NOT add new versioned keys. The manifest is range-based: updating the highest entry automatically covers all CLI versions in that range. +Update the **highest versioned entry** to the new `appkit` version, keeping `"skills": "latest"`. Update `AgentSkillsFallbackVersion` in `internal/build/clicompat.go` to the tested skills version. Do NOT add new versioned keys. The manifest is range-based: updating the highest entry automatically covers all CLI versions in that range. -Write the updated `internal/build/cli-compat.json`. +Write the updated `internal/build/cli-compat.json` and `internal/build/clicompat.go`. ### Step 4b: Breaking changes (add new entry) Ask the user for the **minimum CLI version** that supports the new features. -Add a new entry keyed to that CLI version with the new appkit and skills versions. Keep older entries unchanged so older CLI binaries stay compatible. +Add a new entry keyed to that CLI version with the new `appkit` and `"skills": "latest"`. Update `AgentSkillsFallbackVersion` in `internal/build/clicompat.go` to the tested skills version. If skills compatibility changed, replace `"latest"` in the affected older ranges with the last compatible release. Preserve unrelated AppKit and skills compatibility boundaries. -Write the updated `internal/build/cli-compat.json`. +Write the updated `internal/build/cli-compat.json` and `internal/build/clicompat.go`. ### Step 5: Validate Run the Go tests to ensure the manifest is well-formed: ```bash -go test ./libs/clicompat/... -run TestEmbeddedManifest -v +go test ./libs/clicompat/... -run 'Test(EmbeddedManifest|ResolveEmbeddedAgentSkillsVersion)' -v ``` If validation fails, show the errors and fix them before proceeding. @@ -107,25 +107,25 @@ If validation fails, show the errors and fix them before proceeding. ```bash # Create a new branch from the current branch (or main) -git checkout -b bump-cli-compat-appkit-{appkit_version}-skills-{skills_version} +git checkout -b bump-cli-compat-appkit-{appkit_version}-skills-fallback-{skills_version} # Stage and commit -git add internal/build/cli-compat.json -git commit -s -m "Bump cli-compat to appkit {appkit_version}, skills {skills_version}" +git add internal/build/cli-compat.json internal/build/clicompat.go +git commit -s -m "Bump cli-compat to appkit {appkit_version}, skills fallback {skills_version}" # Push and create PR git push -u origin HEAD gh pr create \ - --title "Bump cli-compat to appkit {appkit_version}, skills {skills_version}" \ + --title "Bump cli-compat to appkit {appkit_version}, skills fallback {skills_version}" \ --body "$(cat <<'EOF' ## Summary Bump `cli-compat.json` to use: - AppKit `{appkit_version}` -- Agent Skills `{skills_version}` +- Agent Skills track latest, with compiled fallback `{skills_version}` ## Checklist - [ ] Evals passed with no regressions -- [ ] `go test ./libs/clicompat/... -run TestEmbeddedManifest` passes +- [ ] `go test ./libs/clicompat/... -run 'Test(EmbeddedManifest|ResolveEmbeddedAgentSkillsVersion)'` passes EOF )" ``` diff --git a/.nextchanges/cli/aitools-latest-skills.md b/.nextchanges/cli/aitools-latest-skills.md new file mode 100644 index 00000000000..4db6c849610 --- /dev/null +++ b/.nextchanges/cli/aitools-latest-skills.md @@ -0,0 +1 @@ +* Fix `databricks aitools install` overwriting newer raw skills with an older pinned release. CLI 1.22.0 and newer now install the latest published skills and prevent automatic downgrades. ([#7069](https://github.com/databricks/cli/pull/7069)) diff --git a/internal/build/README.md b/internal/build/README.md index 913ae7e5f4e..0adf8c77a22 100644 --- a/internal/build/README.md +++ b/internal/build/README.md @@ -2,23 +2,37 @@ `cli-compat.json` maps Databricks CLI versions to compatible AppKit and Agent Skills versions. The CLI uses this manifest to determine which template version to use for `apps init` and which skills version to use for `aitools install`. +Raw skills track the latest published skills release for the newest CLI range. Plugin installs use the agent's marketplace commands without a version pin; the manifest's skills settings only control raw-file delivery. + ## Manifest format ```json { - "0.296.0": { "appkit": "0.27.0", "skills": "0.1.5" }, - "0.290.0": { "appkit": "0.24.0", "skills": "0.1.4" }, - "0.280.0": { "appkit": "0.20.0", "skills": "0.1.0" } + "1.22.0": { "appkit": "0.84.0", "skills": "latest" }, + "1.20.0": { "appkit": "0.84.0", "skills": "0.2.30" }, + "1.0.0": { "appkit": "0.76.1", "skills": "0.2.10" }, + "0.299.2": { "appkit": "0.24.0", "skills": "0.1.5" } } ``` Each key is a CLI version in semver format. Each entry defines a **range floor**: it applies to that CLI version and all versions above it, up to (but not including) the next entry. The manifest should be **sparse** — not every CLI version needs its own entry. Only add a new entry when a compatibility boundary changes. +`skills` is an exact pin for its CLI range or `"latest"` to install the skills repository's latest published GitHub release. When the latest-release lookup fails or the selected skills manifest is missing, ranges tracking latest use `AgentSkillsFallbackVersion` from `internal/build/clicompat.go`. Pinned ranges use their embedded manifest's skills version as the fallback. Changing either fallback takes effect in the next CLI build. Released binaries keep the fallback they shipped with. + +This skills fallback only applies to raw-file delivery. Version checks omit update availability when latest is unknown, and plugin operations don't substitute the fallback version. + +Raw-skills updates and reinstalls reject automatic downgrades, including with `--force`: the fallback must be at least as new as the installed release. Existing installs without a comparable release require a successful latest-release fetch. Explicit `DATABRICKS_SKILLS_REF` overrides and compatibility pins can still select an older release. + +CLIs before `1.6.0` reject the whole remote manifest when any entry uses `"latest"`, even an entry they never resolve to. They use a valid cached manifest or their embedded manifest without returning a parse error. These CLIs stop receiving remote compatibility updates for both AppKit and skills. + +Latest tracking starts at the CLI `1.22.0` range. CLI `1.20.x` and `1.21.x` stay pinned. + For example, with the manifest above: -- CLI `0.285.0` → uses `0.280.0` entry (appkit `0.20.0`) -- CLI `0.293.0` → uses `0.290.0` entry (appkit `0.24.0`) -- CLI `0.300.0` → uses `0.296.0` entry (appkit `0.27.0`, highest versioned) -- CLI `0.0.0-dev+abc` → uses `0.296.0` entry (dev builds use the highest versioned entry) +- CLI `0.299.5` → uses `0.299.2` entry (appkit `0.24.0`) +- CLI `1.19.0` → uses `1.0.0` entry (appkit `0.76.1`) +- CLI `1.21.0` → uses `1.20.0` entry (appkit `0.84.0`, skills `0.2.30`) +- CLI `1.22.0` → uses `1.22.0` entry (appkit `0.84.0`, latest skills with compiled fallback `0.2.30`) +- CLI `0.0.0-dev+abc` → uses `1.22.0` entry (dev builds use the highest versioned entry) ## How the CLI resolves versions @@ -41,12 +55,14 @@ Set `DATABRICKS_FORCE_EMBEDDED_COMPAT=true` to skip all tiers and use only the e ## When to update -The goal is to **keep the manifest sparse** — only add entries at compatibility boundaries. After each AppKit or Agent Skills release: +The goal is to **keep the manifest sparse** — only add entries at compatibility boundaries. Routine skills releases do not require a manifest update: keep `"skills": "latest"` on the newest range. + +When updating AppKit or recording a tested skills fallback: 1. **Run evals** on the new AppKit version. If there is no regression, proceed. 2. **Open a PR** to update `cli-compat.json`. The change depends on the type of release: - - **No breaking changes** (the new AppKit/skills version works with all existing CLI versions): update the existing highest versioned entry's appkit/skills values in-place. Do NOT add a new versioned key. All CLI versions in that range automatically pick up the new versions. - - **Breaking changes** (the new AppKit templates require specific `apps init` features, or the new skills version requires CLI commands that older CLIs lack): add a new entry keyed to the **minimum CLI version** that supports the new features. Older entries keep their previous appkit/skills values so older CLI binaries stay compatible. + - **No breaking changes**: update the highest entry's `appkit` in-place, keeping `"skills": "latest"`. To record a tested skills fallback, update `AgentSkillsFallbackVersion` in `internal/build/clicompat.go`. Do NOT add a new versioned key. + - **Breaking changes** (the new AppKit templates require specific `apps init` features, or the new skills version requires CLI commands that older CLIs lack): add a new entry keyed to the **minimum CLI version** that supports the new features. Set `"skills": "latest"` on the new entry and update the compiled skills fallback to a tested release. If skills compatibility changed, replace `"latest"` in the affected older ranges with the last compatible release. Preserve unrelated AppKit and skills compatibility boundaries. This process is manual for now but can be automated as part of the release workflow in the future. Use the `/bump-cli-compat` Claude Code skill to automate the update and PR creation. @@ -58,7 +74,7 @@ The manifest is validated by Go tests in `libs/clicompat/`: go test ./libs/clicompat/... -run TestEmbeddedManifest -v ``` -This checks: valid JSON, at least one entry, valid semver keys, valid semver entry values, and ascending key order. +This checks: valid JSON, at least one entry, valid semver keys and AppKit versions, concrete skills versions or `"latest"`, a concrete compiled skills fallback, and latest tracking on the newest CLI range. ## Pruning policy diff --git a/internal/build/cli-compat.json b/internal/build/cli-compat.json index 9b39bce748d..7126324cc42 100644 --- a/internal/build/cli-compat.json +++ b/internal/build/cli-compat.json @@ -1,4 +1,5 @@ { + "1.22.0": { "appkit": "0.84.0", "skills": "latest" }, "1.20.0": { "appkit": "0.84.0", "skills": "0.2.30" }, "1.0.0": { "appkit": "0.76.1", "skills": "0.2.10" }, "0.299.2": { "appkit": "0.24.0", "skills": "0.1.5" } diff --git a/internal/build/clicompat.go b/internal/build/clicompat.go index 22cb8f13a03..3142eee1b22 100644 --- a/internal/build/clicompat.go +++ b/internal/build/clicompat.go @@ -2,6 +2,10 @@ package build import _ "embed" +// AgentSkillsFallbackVersion is the tested release used when fetching the latest +// skills release fails. Updating it requires a new CLI build. +const AgentSkillsFallbackVersion = "0.2.30" + // CLICompatManifestJSON is the cli-compat.json manifest embedded at compile time. // Used as the last-resort fallback when both remote fetch and local cache fail. // diff --git a/libs/aitools/installer/dump.go b/libs/aitools/installer/dump.go index e9a1083b93e..4d7ced91361 100644 --- a/libs/aitools/installer/dump.go +++ b/libs/aitools/installer/dump.go @@ -16,7 +16,7 @@ import ( // the cli-compat version pin, --skills cherry-picks, and --experimental exactly // as the agent install path does, since it reuses resolveSkills. func DumpSkillsToPath(ctx context.Context, src ManifestSource, destDir string, opts InstallOptions) (int, error) { - ref, explicit, err := GetSkillsRef(ctx) + ref, explicit, err := getSkillsRef(ctx, true) if err != nil { return 0, err } diff --git a/libs/aitools/installer/dump_test.go b/libs/aitools/installer/dump_test.go index 72abe74a9d0..c73601d250b 100644 --- a/libs/aitools/installer/dump_test.go +++ b/libs/aitools/installer/dump_test.go @@ -1,6 +1,7 @@ package installer import ( + "errors" "io/fs" "os" "path/filepath" @@ -50,3 +51,18 @@ func TestDumpSkillsToPathCherryPick(t *testing.T) { _, err = os.Stat(filepath.Join(dest, "databricks-jobs")) assert.ErrorIs(t, err, fs.ErrNotExist) } + +func TestDumpSkillsToPathFallsBackWhenLatestUnknown(t *testing.T) { + ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) + setupFetchMock(t) + src, fallbackRef := setupSkillsFallback(t, errors.New("network down")) + dest := filepath.Join(t.TempDir(), "out") + + n, err := DumpSkillsToPath(ctx, src, dest, InstallOptions{}) + require.NoError(t, err) + assert.Equal(t, 2, n) + assert.Contains(t, stderr.String(), "Using skills version "+DisplaySkillsVersion(fallbackRef)) + for _, name := range []string{"databricks-sql", "databricks-jobs"} { + assert.FileExists(t, filepath.Join(dest, name, "SKILL.md")) + } +} diff --git a/libs/aitools/installer/installer.go b/libs/aitools/installer/installer.go index e3f39e09435..77cc44dfe1b 100644 --- a/libs/aitools/installer/installer.go +++ b/libs/aitools/installer/installer.go @@ -92,35 +92,42 @@ var latestSkillsReleaseRefFn = fetchLatestSkillsReleaseRef const skillsLatestSentinel = clicompat.AgentSkillsLatest // GetSkillsRef returns the skills repo ref to use and whether it was explicitly -// pinned (as opposed to tracking latest). +// pinned (as opposed to tracking latest). An unresolved latest release is an error. // // By default we track the latest skills release tag, not the skills repo's main -// branch. cli-compat.json is the remote control for this: it normally reports -// "latest", but if a future change makes a skills release incompatible with -// older CLIs, it can be edited remotely to pin those CLIs to the last compatible -// version with no CLI release. DATABRICKS_SKILLS_REF overrides everything for -// eval/reproducibility. (AppKit version resolution is unaffected; cli-compat -// still manages it via its own field.) +// branch. cli-compat.json is the remote control for this: its newest range +// normally tracks latest, but if a future change makes a skills release +// incompatible with older CLIs, it can be edited remotely to pin those CLIs to +// the last compatible version with no CLI release. DATABRICKS_SKILLS_REF +// overrides everything for eval/reproducibility. (AppKit version resolution is +// unaffected; cli-compat still manages it via its own field.) func GetSkillsRef(ctx context.Context) (ref string, explicit bool, err error) { + return getSkillsRef(ctx, false) +} + +func getSkillsRef(ctx context.Context, allowFallback bool) (ref string, explicit bool, err error) { if ref := env.Get(ctx, "DATABRICKS_SKILLS_REF"); ref != "" { return ref, true, nil } v, err := clicompat.ResolveAgentSkillsVersion(ctx) if err != nil { log.Debugf(ctx, "could not resolve skills compatibility, resolving latest release: %v", err) - return resolveLatestSkillsReleaseRef(ctx) + return resolveLatestSkillsReleaseRef(ctx, allowFallback) } if v == "" || v == skillsLatestSentinel { - return resolveLatestSkillsReleaseRef(ctx) + return resolveLatestSkillsReleaseRef(ctx, allowFallback) } return "v" + v, true, nil } -func resolveLatestSkillsReleaseRef(ctx context.Context) (ref string, explicit bool, err error) { +func resolveLatestSkillsReleaseRef(ctx context.Context, allowFallback bool) (ref string, explicit bool, err error) { ref, err = latestSkillsReleaseRefFn(ctx) if err == nil { return ref, false, nil } + if !allowFallback { + return "", false, fmt.Errorf("resolve latest skills release: %w", err) + } embedded, embeddedErr := clicompat.ResolveEmbeddedAgentSkillsVersion() if embeddedErr == nil && embedded != "" && embedded != skillsLatestSentinel { @@ -267,25 +274,56 @@ func FetchSkillsManifestWithFallback(ctx context.Context, src ManifestSource, re return manifest, ref, err } -// InstallSkillsForAgents fetches the manifest and installs skills for the given agents. -// This is the core installation function. Callers are responsible for agent detection, -// prompting, and printing the "Installing..." header. -func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgents []*agents.Agent, opts InstallOptions) error { - ref, explicit, err := GetSkillsRef(ctx) - if err != nil { - return err +// installedSkillsRelease returns the newest recorded raw-skills release and +// whether fallback is safe to consider. Branch refs cannot be ordered, so they +// require a successful latest-release fetch instead of an embedded fallback. +func installedSkillsRelease(state *InstallState) (ref string, allowFallback bool) { + if state == nil || len(state.Skills) == 0 { + return "", true + } + ref = semver.Canonical("v" + DisplaySkillsVersion(state.Release)) + allowFallback = ref != "" + // Plugin installs can overwrite state.Release, and selective skill installs + // can leave files from a newer release. Keep their provenance in the check. + knownSkills := make(map[string]bool, len(state.Skills)) + for path, record := range state.Files { + if record.Origin == "" { + continue + } + origin := semver.Canonical("v" + DisplaySkillsVersion(record.Origin)) + if origin == "" { + allowFallback = false + } else { + name, _, _ := strings.Cut(path, "/") + knownSkills[name] = true + } + if semver.Compare(origin, ref) > 0 { + ref = origin + } } - cmdio.LogProgress(ctx, "Using skills version "+DisplaySkillsVersion(ref)) - cmdio.LogProgress(ctx, "Fetching skills manifest...") - manifest, ref, err := FetchSkillsManifestWithFallback(ctx, src, ref, !explicit) - if err != nil { - return err + // Without raw-file provenance, a mixed legacy/plugin install has no + // reliable raw-skills release to compare with the fallback. + if len(state.Plugins) > 0 { + for name := range state.Skills { + if !knownSkills[name] { + allowFallback = false + } + } } + return ref, allowFallback +} - if opts.IncludeExperimental && !manifestHasExperimental(manifest) { - log.Warnf(ctx, "--experimental was set but the manifest at %s exposes no experimental skills. Set DATABRICKS_SKILLS_REF to a release or ref that includes them.", ref) +func checkSkillsDowngrade(installedRef, ref string, explicit bool) error { + if !explicit && semver.Compare(ref, installedRef) < 0 { + return fmt.Errorf("refusing to downgrade skills from %s to %s; retry when the latest release is available or set DATABRICKS_SKILLS_REF=%s to select this version explicitly", DisplaySkillsVersion(installedRef), DisplaySkillsVersion(ref), ref) } + return nil +} +// InstallSkillsForAgents fetches the manifest and installs skills for the given agents. +// This is the core installation function. Callers are responsible for agent detection, +// prompting, and printing the "Installing..." header. +func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgents []*agents.Agent, opts InstallOptions) error { scope := opts.Scope if scope == "" { scope = ScopeGlobal @@ -310,7 +348,7 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent } } - // Load existing state for idempotency checks. + // Load existing state for idempotency and downgrade checks. state, err := LoadState(baseDir) if err != nil { return fmt.Errorf("failed to load install state: %w", err) @@ -323,10 +361,31 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent // full install first, which rebuilds complete state. Global scope only, since // legacy installs only ever wrote to the global dir. // hasLegacyInstall lives in update.go (shared with the update/uninstall paths). - if state == nil && scope == ScopeGlobal && len(opts.SpecificSkills) > 0 && hasLegacyInstall(ctx, baseDir) { + legacyInstall := state == nil && scope == ScopeGlobal && hasLegacyInstall(ctx, baseDir) + if legacyInstall && len(opts.SpecificSkills) > 0 { return errors.New("legacy install detected without state tracking; run 'databricks aitools install' (without a skill name) first to rebuild state") } + installedRef, allowFallback := installedSkillsRelease(state) + allowFallback = allowFallback && !legacyInstall + ref, explicit, err := getSkillsRef(ctx, allowFallback) + if err != nil { + return err + } + cmdio.LogProgress(ctx, "Using skills version "+DisplaySkillsVersion(ref)) + cmdio.LogProgress(ctx, "Fetching skills manifest...") + manifest, ref, err := FetchSkillsManifestWithFallback(ctx, src, ref, allowFallback && !explicit) + if err != nil { + return err + } + if err := checkSkillsDowngrade(installedRef, ref, explicit); err != nil { + return err + } + + if opts.IncludeExperimental && !manifestHasExperimental(manifest) { + log.Warnf(ctx, "--experimental was set but the manifest at %s exposes no experimental skills. Set DATABRICKS_SKILLS_REF to a release or ref that includes them.", ref) + } + // Filter skills based on options, experimental flag, and CLI version. targetSkills, err := resolveSkills(ctx, manifest.Skills, opts) if err != nil { diff --git a/libs/aitools/installer/installer_test.go b/libs/aitools/installer/installer_test.go index b7f74630757..8b46d8b0901 100644 --- a/libs/aitools/installer/installer_test.go +++ b/libs/aitools/installer/installer_test.go @@ -3,10 +3,13 @@ package installer import ( "bytes" "context" + "encoding/json" "errors" "fmt" "io/fs" "log/slog" + "net/http" + "net/http/httptest" "os" "path/filepath" "sync" @@ -16,6 +19,7 @@ import ( "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/aitools/agents" + "github.com/databricks/cli/libs/clicompat" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/log" "github.com/stretchr/testify/assert" @@ -446,6 +450,274 @@ func TestInstallSkillsForAgentsWritesState(t *testing.T) { assert.Contains(t, stderr.String(), "Installed 2 skills.") } +func setupSkillsFallback(t *testing.T, latestErr error) (ManifestSource, string) { + t.Helper() + setBuildVersion(t, "1.22.0") + t.Setenv("DATABRICKS_SKILLS_REF", "") + t.Setenv("DATABRICKS_FORCE_EMBEDDED_COMPAT", "false") + t.Setenv("DATABRICKS_CACHE_ENABLED", "true") + cacheDir := t.TempDir() + t.Setenv("DATABRICKS_CACHE_DIR", cacheDir) + require.NoError(t, os.WriteFile(filepath.Join(cacheDir, "compat-manifest.json"), build.CLICompatManifestJSON, 0o600)) + stubLatestSkillsReleaseRef(t, "v9.9.9", latestErr) + fallbackRef := "v" + build.AgentSkillsFallbackVersion + body, err := json.Marshal(testManifest()) + require.NoError(t, err) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/"+fallbackRef+"/manifest.json" { + w.WriteHeader(http.StatusNotFound) + return + } + _, _ = w.Write(body) + })) + t.Cleanup(srv.Close) + t.Setenv("DATABRICKS_SKILLS_BASE_URL", srv.URL) + return &GitHubManifestSource{}, fallbackRef +} + +func testSkillsFallbackVersions(t *testing.T, run func(context.Context, ManifestSource, []*agents.Agent) error) { + t.Helper() + for _, failure := range []struct { + name string + latestErr error + }{ + {name: "release lookup failed", latestErr: errors.New("HTTP 403 fetching latest skills release")}, + {name: "latest manifest missing"}, + } { + t.Run(failure.name, func(t *testing.T) { + for _, tc := range []struct { + name string + installedRef string + pluginInstall bool + legacyState bool + unknown bool + downgrade bool + }{ + {name: "newer fallback", installedRef: "v0.0.1"}, + {name: "equal fallback"}, + {name: "older fallback", installedRef: "v9.9.8", downgrade: true}, + {name: "older fallback after plugin install", installedRef: "v9.9.8", pluginInstall: true, downgrade: true}, + {name: "unknown installed release", installedRef: "main", unknown: true}, + {name: "unknown release after plugin install", installedRef: "main", pluginInstall: true, unknown: true}, + {name: "legacy state after plugin install", installedRef: "v9.9.8", pluginInstall: true, legacyState: true, unknown: true}, + } { + t.Run(tc.name, func(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + src, fallbackRef := setupSkillsFallback(t, failure.latestErr) + installedRef := tc.installedRef + if installedRef == "" { + installedRef = fallbackRef + } + t.Setenv("DATABRICKS_SKILLS_REF", installedRef) + targetAgents := []*agents.Agent{testAgent(tmp)} + installedManifest := testManifest() + switch { + case tc.downgrade || tc.unknown: + installedManifest.Skills["databricks-sql"] = SkillMeta{Version: "0.2.0", Files: []string{"SKILL.md"}} + installedManifest.Skills["databricks-new"] = SkillMeta{Version: "0.1.0", Files: []string{"SKILL.md"}} + case tc.installedRef != "": + installedManifest.Skills["databricks-sql"] = SkillMeta{Version: "0.0.9", Files: []string{"SKILL.md"}} + } + require.NoError(t, InstallSkillsForAgents(ctx, &mockManifestSource{manifest: installedManifest}, targetAgents, InstallOptions{})) + baseDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + if tc.legacyState { + state, err := LoadState(baseDir) + require.NoError(t, err) + state.Files = nil + require.NoError(t, SaveState(baseDir, state)) + } + if tc.pluginInstall { + require.NoError(t, RecordPluginInstalls(ctx, ScopeGlobal, map[string]PluginRecord{ + "claude-code": {Plugin: "databricks", Version: DisplaySkillsVersion(fallbackRef)}, + }, fallbackRef)) + } + statePath := filepath.Join(baseDir, stateFileName) + before, err := os.ReadFile(statePath) + require.NoError(t, err) + + t.Setenv("DATABRICKS_SKILLS_REF", "") + err = run(ctx, src, targetAgents) + switch { + case tc.downgrade: + require.ErrorContains(t, err, "refusing to downgrade skills from 9.9.8 to "+DisplaySkillsVersion(fallbackRef)) + require.ErrorContains(t, err, "DATABRICKS_SKILLS_REF="+fallbackRef) + case tc.unknown && failure.latestErr != nil: + require.ErrorIs(t, err, failure.latestErr) + case tc.unknown: + require.ErrorIs(t, err, clicompat.ErrNotFound) + default: + require.NoError(t, err) + state, err := LoadState(baseDir) + require.NoError(t, err) + assert.Equal(t, fallbackRef, state.Release) + assert.Equal(t, map[string]string{"databricks-sql": "0.1.0", "databricks-jobs": "0.1.0"}, state.Skills) + return + } + + after, err := os.ReadFile(statePath) + require.NoError(t, err) + assert.Equal(t, before, after, "rejected fallback must not rewrite install state") + for name := range installedManifest.Skills { + for _, dir := range []string{baseDir, filepath.Join(tmp, ".test-agent", "skills")} { + content, err := os.ReadFile(filepath.Join(dir, name, "SKILL.md")) + require.NoError(t, err) + assert.Equal(t, "# "+name+"/SKILL.md", string(content)) + } + } + }) + } + }) + } +} + +func TestInstallSkillsForAgentsFallbackVersions(t *testing.T) { + testSkillsFallbackVersions(t, func(ctx context.Context, src ManifestSource, targetAgents []*agents.Agent) error { + return InstallSkillsForAgents(ctx, src, targetAgents, InstallOptions{}) + }) +} + +func testSkillsRefSelection(t *testing.T, run func(context.Context, ManifestSource, []*agents.Agent) error) { + t.Helper() + for _, tc := range []struct { + name string + installedRef string + selectedRef string + envPin bool + compatPin bool + wantErr bool + }{ + {name: "environment pin", installedRef: "v9.9.8", selectedRef: testSkillsRef, envPin: true}, + {name: "compatibility pin", installedRef: "v9.9.8", selectedRef: testSkillsRef, compatPin: true}, + {name: "latest after branch", installedRef: "main", selectedRef: "v9.9.9"}, + {name: "older latest release", installedRef: "v9.9.8", selectedRef: testSkillsRef, wantErr: true}, + } { + t.Run(tc.name, func(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", tc.installedRef) + targetAgents := []*agents.Agent{testAgent(tmp)} + installedManifest := testManifest() + version := "0.2.0" + if tc.installedRef == "main" { + version = "0.0.9" + } + installedManifest.Skills["databricks-sql"] = SkillMeta{Version: version, Files: []string{"SKILL.md"}} + require.NoError(t, InstallSkillsForAgents(ctx, &mockManifestSource{manifest: installedManifest}, targetAgents, InstallOptions{})) + + t.Setenv("DATABRICKS_SKILLS_REF", "") + switch { + case tc.envPin: + t.Setenv("DATABRICKS_SKILLS_REF", tc.selectedRef) + case tc.compatPin: + withCachedCompat(t, DisplaySkillsVersion(tc.selectedRef)) + default: + withCachedCompat(t, skillsLatestSentinel) + } + stubLatestSkillsReleaseRef(t, tc.selectedRef, nil) + err := run(ctx, &mockManifestSource{manifest: testManifest()}, targetAgents) + if tc.wantErr { + require.ErrorContains(t, err, "refusing to downgrade skills") + } else { + require.NoError(t, err) + } + state, err := LoadState(filepath.Join(tmp, ".databricks", "aitools", "skills")) + require.NoError(t, err) + if tc.wantErr { + assert.Equal(t, tc.installedRef, state.Release) + assert.Equal(t, version, state.Skills["databricks-sql"]) + } else { + assert.Equal(t, tc.selectedRef, state.Release) + assert.Equal(t, "0.1.0", state.Skills["databricks-sql"]) + } + }) + } +} + +func TestInstallSkillsForAgentsRefSelection(t *testing.T) { + testSkillsRefSelection(t, func(ctx context.Context, src ManifestSource, targetAgents []*agents.Agent) error { + return InstallSkillsForAgents(ctx, src, targetAgents, InstallOptions{}) + }) +} + +func TestCheckSkillsDowngrade(t *testing.T) { + for _, tc := range []struct { + name string + installedRef string + ref string + wantErr bool + }{ + {name: "fresh install", ref: "v0.2.30"}, + {name: "same release", installedRef: "v0.2.30", ref: "v0.2.30"}, + {name: "numeric upgrade", installedRef: "v0.2.9", ref: "v0.2.30"}, + {name: "numeric downgrade", installedRef: "v0.2.30", ref: "v0.2.9", wantErr: true}, + {name: "prerelease upgrade", installedRef: "v0.2.30-rc.1", ref: "v0.2.30"}, + {name: "prerelease downgrade", installedRef: "v0.2.30", ref: "v0.2.30-rc.1", wantErr: true}, + {name: "build metadata", installedRef: "v0.2.30+test", ref: "v0.2.30"}, + } { + t.Run(tc.name, func(t *testing.T) { + err := checkSkillsDowngrade(tc.installedRef, tc.ref, false) + if tc.wantErr { + require.ErrorContains(t, err, "refusing to downgrade skills") + } else { + require.NoError(t, err) + } + }) + } +} + +func TestInstallSkillsForAgentsFallbackForFreshInstall(t *testing.T) { + for _, pluginOnly := range []bool{false, true} { + for _, missingManifest := range []bool{false, true} { + t.Run(fmt.Sprintf("pluginOnly=%t/missingManifest=%t", pluginOnly, missingManifest), func(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + if pluginOnly { + require.NoError(t, RecordPluginInstalls(ctx, ScopeGlobal, map[string]PluginRecord{ + "claude-code": {Plugin: "databricks", Version: "9.9.8"}, + }, "v9.9.8")) + } + var latestErr error + if !missingManifest { + latestErr = errors.New("HTTP 403 fetching latest skills release") + } + src, fallbackRef := setupSkillsFallback(t, latestErr) + require.NoError(t, InstallSkillsForAgents(ctx, src, []*agents.Agent{testAgent(tmp)}, InstallOptions{})) + state, err := LoadState(filepath.Join(tmp, ".databricks", "aitools", "skills")) + require.NoError(t, err) + require.NotNil(t, state) + assert.Equal(t, fallbackRef, state.Release) + assert.Len(t, state.Skills, 2) + if pluginOnly { + assert.Contains(t, state.Plugins, "claude-code") + } + }) + } + } +} + +func TestInstallSkillsForAgentsPreservesLegacySkillsOnFallback(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + baseDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + skillDir := filepath.Join(baseDir, "databricks-sql") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + skillPath := filepath.Join(skillDir, "SKILL.md") + require.NoError(t, os.WriteFile(skillPath, []byte("installed skill"), 0o644)) + latestErr := errors.New("network down") + src, _ := setupSkillsFallback(t, latestErr) + + err := InstallSkillsForAgents(ctx, src, []*agents.Agent{testAgent(tmp)}, InstallOptions{}) + require.ErrorIs(t, err, latestErr) + content, err := os.ReadFile(skillPath) + require.NoError(t, err) + assert.Equal(t, "installed skill", string(content)) + assert.NoFileExists(t, filepath.Join(baseDir, stateFileName)) +} + func TestInstallPurgesStaleFileRecordsOnRefetch(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) @@ -1055,15 +1327,119 @@ func TestGetSkillsRefDefaultsToLatest(t *testing.T) { assert.False(t, explicit, "tracking latest is not an explicit pin") } -func TestGetSkillsRefLatestReleaseFallsBackToEmbeddedPin(t *testing.T) { +func TestGetSkillsRefDefaultManifest(t *testing.T) { + var m clicompat.Manifest + require.NoError(t, json.Unmarshal(build.CLICompatManifestJSON, &m)) + + // The table only records which CLI versions track latest. Pins come from + // the embedded manifest so routine skills bumps need no test changes. + tests := []struct { + version string + latest bool + }{ + {version: "0.299.2"}, + {version: "1.0.0"}, + {version: "1.19.9"}, + {version: "1.20.0"}, + {version: "1.21.0"}, + {version: "1.21.9"}, + {version: "1.22.0", latest: true}, + {version: "1.23.0", latest: true}, + {version: "0.0.0-dev+test", latest: true}, + } + for _, tc := range tests { + entry, err := clicompat.Resolve(m, tc.version) + require.NoError(t, err) + pinnedRef := "v" + entry.AgentSkills + + for _, offline := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/offline=%t", tc.version, offline), func(t *testing.T) { + t.Setenv("DATABRICKS_SKILLS_REF", "") + setBuildVersion(t, tc.version) + cacheDir := t.TempDir() + t.Setenv("DATABRICKS_CACHE_DIR", cacheDir) + require.NoError(t, os.WriteFile(filepath.Join(cacheDir, "compat-manifest.json"), build.CLICompatManifestJSON, 0o600)) + wantRef := pinnedRef + var latestErr error + if offline { + latestErr = errors.New("network down") + } + switch { + case tc.latest && offline: + wantRef = "" + case tc.latest: + wantRef = "v9.9.9" + } + stubLatestSkillsReleaseRef(t, "v9.9.9", latestErr) + + ref, explicit, err := GetSkillsRef(t.Context()) + if tc.latest && offline { + require.ErrorIs(t, err, latestErr) + } else { + require.NoError(t, err) + } + assert.Equal(t, wantRef, ref) + assert.Equal(t, !tc.latest, explicit) + }) + } + } +} + +func TestGetSkillsRefLatestReleaseLookupFails(t *testing.T) { + setBuildVersion(t, "1.22.0") t.Setenv("DATABRICKS_SKILLS_REF", "") withCachedCompat(t, skillsLatestSentinel) - stubLatestSkillsReleaseRef(t, "", errors.New("network down")) + latestErr := errors.New("network down") + stubLatestSkillsReleaseRef(t, "", latestErr) ref, explicit, err := GetSkillsRef(t.Context()) - require.NoError(t, err) - assert.Equal(t, "v0.2.30", ref) - assert.False(t, explicit, "an embedded fallback is not a user pin") + require.ErrorIs(t, err, latestErr) + assert.Empty(t, ref, "the embedded fallback is not the latest release") + assert.False(t, explicit) +} + +func TestFetchSkillsManifestWithFallback(t *testing.T) { + const missingRef = "v9.9.9" + setBuildVersion(t, "1.22.0") + fallbackRef := "v" + build.AgentSkillsFallbackVersion + + tests := []struct { + name string + status int + allowFallback bool + wantRef string + wantErr bool + }{ + {name: "latest manifest missing", status: http.StatusNotFound, allowFallback: true, wantRef: fallbackRef}, + {name: "explicit pin missing", status: http.StatusNotFound, wantRef: missingRef, wantErr: true}, + {name: "server error", status: http.StatusInternalServerError, allowFallback: true, wantRef: missingRef, wantErr: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + body, err := json.Marshal(testManifest()) + require.NoError(t, err) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/"+fallbackRef+"/manifest.json" { + w.WriteHeader(tc.status) + return + } + _, _ = w.Write(body) + })) + defer srv.Close() + t.Setenv("DATABRICKS_SKILLS_BASE_URL", srv.URL) + + manifest, ref, err := FetchSkillsManifestWithFallback( + cmdio.MockDiscard(t.Context()), &GitHubManifestSource{}, missingRef, tc.allowFallback, + ) + assert.Equal(t, tc.wantRef, ref) + if tc.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, "1", manifest.Version) + }) + } } func TestGetSkillsRefEnvEscapeHatch(t *testing.T) { diff --git a/libs/aitools/installer/update.go b/libs/aitools/installer/update.go index baa3d6a9863..5fa183e0eae 100644 --- a/libs/aitools/installer/update.go +++ b/libs/aitools/installer/update.go @@ -86,7 +86,8 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent return nil, errors.New("no skills or plugins installed. Run 'databricks aitools install' to install") } - latestTag, explicit, err := GetSkillsRef(ctx) + installedRef, allowFallback := installedSkillsRelease(state) + latestTag, explicit, err := getSkillsRef(ctx, allowFallback) if err != nil { return nil, err } @@ -99,7 +100,7 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent return &UpdateResult{Unchanged: slices.Sorted(maps.Keys(state.Skills))}, nil } - manifest, latestTag, err := FetchSkillsManifestWithFallback(ctx, src, latestTag, !explicit) + manifest, latestTag, err := FetchSkillsManifestWithFallback(ctx, src, latestTag, allowFallback && !explicit) if err != nil { if opts.Check { log.Warnf(ctx, "Could not fetch manifest: %v", err) @@ -107,6 +108,9 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent } return nil, err } + if err := checkSkillsDowngrade(installedRef, latestTag, explicit); err != nil { + return nil, err + } // Determine the skill set to consider. skillSet := buildUpdateSkillSet(state, manifest, opts) diff --git a/libs/aitools/installer/update_test.go b/libs/aitools/installer/update_test.go index 53acc9aa85a..2a9326e4f82 100644 --- a/libs/aitools/installer/update_test.go +++ b/libs/aitools/installer/update_test.go @@ -66,6 +66,28 @@ func TestUpdateAlreadyUpToDate(t *testing.T) { assert.Empty(t, result.Added) } +func TestUpdateSkillsFallbackVersions(t *testing.T) { + for _, force := range []bool{false, true} { + name := "normal" + if force { + name = "force" + } + t.Run(name, func(t *testing.T) { + testSkillsFallbackVersions(t, func(ctx context.Context, src ManifestSource, targetAgents []*agents.Agent) error { + _, err := UpdateSkills(ctx, src, targetAgents, UpdateOptions{Force: force}) + return err + }) + }) + } +} + +func TestUpdateSkillsRefSelection(t *testing.T) { + testSkillsRefSelection(t, func(ctx context.Context, src ManifestSource, targetAgents []*agents.Agent) error { + _, err := UpdateSkills(ctx, src, targetAgents, UpdateOptions{}) + return err + }) +} + func TestUpdateVersionDiffDetected(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) diff --git a/libs/clicompat/clicompat.go b/libs/clicompat/clicompat.go index 71ffe75d36c..446c8e4755c 100644 --- a/libs/clicompat/clicompat.go +++ b/libs/clicompat/clicompat.go @@ -163,9 +163,9 @@ func ResolveEmbeddedAppKitVersion() (string, error) { return entry.AppKit, nil } -// ResolveEmbeddedAgentSkillsVersion resolves the Agent Skills version from only -// the embedded manifest for the current CLI version. Used as a fallback when the -// primary version points to a non-existent tag. +// ResolveEmbeddedAgentSkillsVersion resolves the fallback for the current CLI +// without fetching compatibility data. Ranges tracking latest use the tested +// release compiled into the binary. func ResolveEmbeddedAgentSkillsVersion() (string, error) { m, err := parseEmbeddedManifest() if err != nil { @@ -175,6 +175,9 @@ func ResolveEmbeddedAgentSkillsVersion() (string, error) { if err != nil { return "", fmt.Errorf("embedded manifest resolve: %w", err) } + if entry.AgentSkills == AgentSkillsLatest { + return build.AgentSkillsFallbackVersion, nil + } return entry.AgentSkills, nil } diff --git a/libs/clicompat/clicompat_test.go b/libs/clicompat/clicompat_test.go index 87d29c6dfe9..8d480dddfc4 100644 --- a/libs/clicompat/clicompat_test.go +++ b/libs/clicompat/clicompat_test.go @@ -297,6 +297,31 @@ func TestResolveEntry_RemoteFailUsesEmbedded(t *testing.T) { assert.NotEmpty(t, entry.AppKit) } +func TestResolveAgentSkillsVersion(t *testing.T) { + tests := []struct { + name string + manifest string + want string + }{ + {name: "pinned", manifest: `{"0.0.0":{"appkit":"0.84.0","skills":"0.2.30"}}`, want: "0.2.30"}, + {name: "latest sentinel", manifest: `{"0.0.0":{"appkit":"0.84.0","skills":"latest"}}`, want: AgentSkillsLatest}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := testContext(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(tc.manifest)) + })) + defer srv.Close() + redirectToServer(t, srv) + + v, err := ResolveAgentSkillsVersion(ctx) + require.NoError(t, err) + assert.Equal(t, tc.want, v) + }) + } +} + // --- ResolveEmbeddedAppKitVersion --- func TestResolveEmbeddedAppKitVersion(t *testing.T) { @@ -306,6 +331,27 @@ func TestResolveEmbeddedAppKitVersion(t *testing.T) { assert.True(t, semver.IsValid("v"+v), "embedded resolved version should be valid semver") } +func TestResolveEmbeddedAgentSkillsVersion(t *testing.T) { + for _, tc := range []struct { + version string + want string + }{ + {version: "1.0.0", want: "0.2.10"}, + {version: "1.22.0", want: build.AgentSkillsFallbackVersion}, + {version: "0.0.0-dev+test", want: build.AgentSkillsFallbackVersion}, + } { + t.Run(tc.version, func(t *testing.T) { + original := build.GetInfo().Version + build.SetBuildVersion(tc.version) + t.Cleanup(func() { build.SetBuildVersion(original) }) + + v, err := ResolveEmbeddedAgentSkillsVersion() + require.NoError(t, err) + assert.Equal(t, tc.want, v) + }) + } +} + // --- Embedded manifest validation (replaces AppKit TS validator) --- func TestEmbeddedManifest_IsWellFormed(t *testing.T) { @@ -325,6 +371,13 @@ func TestEmbeddedManifest_IsWellFormed(t *testing.T) { return semver.Compare("v"+a, "v"+b) }) + // Pinning the newest range (e.g. after a breaking skills release) must + // update this assertion in the same change. + assert.Equal(t, AgentSkillsLatest, m[keys[len(keys)-1]].AgentSkills, + "the newest CLI range must track the latest skills release") + assert.True(t, semver.IsValid("v"+build.AgentSkillsFallbackVersion), + "compiled skills fallback must be a concrete version") + // Keys must be in ascending semver order. for i := 1; i < len(keys); i++ { cmp := semver.Compare("v"+keys[i-1], "v"+keys[i])