From 291f8db9fdd68ef08fa934187d1e71d6b7b6bc47 Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Wed, 29 Apr 2026 15:46:24 +0200 Subject: [PATCH 01/10] feat: resolve AppKit template version from compatibility manifest Replace the hardcoded appkitDefaultVersion with a compatibility manifest that maps CLI versions to compatible AppKit template and Agent Skills versions. At runtime, apps init fetches the latest manifest from the AppKit GitHub repo. If the fetch fails, it falls back to an embedded copy. The resolution algorithm finds the nearest compatible version for the running CLI (exact match, nearest lower, or "next" for newer CLIs). The resolved skills version is passed to the skills installer via the DATABRICKS_SKILLS_REF context variable, ensuring apps init recommends a manifest-compatible skills version. Also add a Makefile target (fetch-compat-manifest) for CI to fetch the latest manifest before building the CLI binary. Signed-off-by: Pawel Kosiec --- Makefile | 7 ++ cmd/apps/init.go | 36 ++++++- libs/apps/compat/cli-compat.json | 7 ++ libs/apps/compat/compat.go | 145 +++++++++++++++++++++++++++ libs/apps/compat/compat_test.go | 166 +++++++++++++++++++++++++++++++ 5 files changed, 357 insertions(+), 4 deletions(-) create mode 100644 libs/apps/compat/cli-compat.json create mode 100644 libs/apps/compat/compat.go create mode 100644 libs/apps/compat/compat_test.go diff --git a/Makefile b/Makefile index 75e7d0860d..3896caa775 100644 --- a/Makefile +++ b/Makefile @@ -134,6 +134,13 @@ showcover: acc-showcover: go tool cover -html=coverage-acceptance.txt +.PHONY: fetch-compat-manifest +fetch-compat-manifest: + @curl -sfL https://raw.githubusercontent.com/databricks/appkit/main/cli-compat.json \ + -o libs/apps/compat/cli-compat.json \ + && echo "Fetched latest cli-compat.json" \ + || echo "Warning: failed to fetch cli-compat.json, using existing copy" + .PHONY: build build: tidy go build diff --git a/cmd/apps/init.go b/cmd/apps/init.go index 6e6ba2ccee..bd38541e29 100644 --- a/cmd/apps/init.go +++ b/cmd/apps/init.go @@ -18,7 +18,9 @@ import ( "github.com/charmbracelet/huh" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/experimental/aitools/lib/agents" + "github.com/databricks/cli/internal/build" "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/databricks/cli/libs/apps/compat" "github.com/databricks/cli/libs/apps/generator" "github.com/databricks/cli/libs/apps/initializer" "github.com/databricks/cli/libs/apps/manifest" @@ -64,6 +66,16 @@ func normalizeVersion(version string) string { return version } +// resolveFromManifest fetches the compatibility manifest and resolves the +// entry for the current CLI version. +func resolveFromManifest(ctx context.Context) (compat.Entry, error) { + m, err := compat.FetchManifest(ctx) + if err != nil { + return compat.Entry{}, err + } + return compat.Resolve(m, build.GetInfo().Version) +} + func newInitCmd() *cobra.Command { var ( templatePath string @@ -768,6 +780,7 @@ func runCreate(ctx context.Context, opts createOptions) error { // Resolve the git reference (branch/tag) to use for default appkit template gitRef := opts.branch usingDefaultTemplate := templateSrc == "" + var resolvedSkillsVersion string if usingDefaultTemplate { // Using default appkit template - resolve version switch { @@ -776,8 +789,15 @@ func runCreate(ctx context.Context, opts createOptions) error { case opts.version != "": gitRef = normalizeVersion(opts.version) default: - // Default: use pinned version - gitRef = appkitDefaultVersion + // Resolve from compatibility manifest (fetch from GitHub, fall back to embedded). + if entry, err := resolveFromManifest(ctx); err != nil { + log.Warnf(ctx, "Manifest resolution failed (%v), using hardcoded default %s", err, appkitDefaultVersion) + gitRef = appkitDefaultVersion + } else { + gitRef = normalizeVersion(entry.Appkit) + resolvedSkillsVersion = entry.Skills + log.Infof(ctx, "Resolved AppKit template version %s (skills %s) from compatibility manifest for CLI %s", entry.Appkit, entry.Skills, build.GetInfo().Version) + } } templateSrc = appkitRepoURL } @@ -1131,13 +1151,21 @@ func runCreate(ctx context.Context, opts createOptions) error { prompt.PrintSetupNotes(ctx, notes) } + // If the manifest resolved a skills version, set it on the context so the + // skills installer uses the manifest-compatible version instead of its + // embedded SKILLS_VERSION default. + skillsCtx := ctx + if resolvedSkillsVersion != "" { + skillsCtx = env.Set(ctx, "DATABRICKS_SKILLS_REF", "v"+resolvedSkillsVersion) + } + // Recommend skills installation if coding agents are detected without skills. // In flags mode, only print a hint — never prompt interactively. if flagsMode { - if !agents.HasDatabricksSkillsInstalled(ctx) { + if !agents.HasDatabricksSkillsInstalled(skillsCtx) { cmdio.LogString(ctx, "Tip: coding agents detected without Databricks skills. Run 'databricks experimental aitools skills install' to install them.") } - } else if err := agents.RecommendSkillsInstall(ctx, installer.InstallAllSkills); err != nil { + } else if err := agents.RecommendSkillsInstall(skillsCtx, installer.InstallAllSkills); err != nil { log.Warnf(ctx, "Skills recommendation failed: %v", err) } diff --git a/libs/apps/compat/cli-compat.json b/libs/apps/compat/cli-compat.json new file mode 100644 index 0000000000..fd13c5f96b --- /dev/null +++ b/libs/apps/compat/cli-compat.json @@ -0,0 +1,7 @@ +{ + "next": { "appkit": "0.27.0", "skills": "0.1.5" }, + "0.296.0": { "appkit": "0.27.0", "skills": "0.1.5" }, + "0.295.0": { "appkit": "0.27.0", "skills": "0.1.5" }, + "0.290.0": { "appkit": "0.24.0", "skills": "0.1.5" }, + "0.288.0": { "appkit": "0.24.0", "skills": "0.1.4" } +} diff --git a/libs/apps/compat/compat.go b/libs/apps/compat/compat.go new file mode 100644 index 0000000000..19b663cbdd --- /dev/null +++ b/libs/apps/compat/compat.go @@ -0,0 +1,145 @@ +package compat + +import ( + "context" + _ "embed" + "encoding/json" + "fmt" + "io" + "net/http" + "sort" + "strings" + "time" + + "github.com/databricks/cli/libs/log" + "golang.org/x/mod/semver" +) + +const ( + // manifestURL is the raw GitHub URL for the compatibility manifest. + manifestURL = "https://raw.githubusercontent.com/databricks/appkit/main/cli-compat.json" + + // fetchTimeout is the HTTP timeout for fetching the manifest at runtime. + fetchTimeout = 5 * time.Second + + // nextKey is the special manifest key for CLI versions newer than any entry. + nextKey = "next" +) + +// Entry maps a CLI version to compatible AppKit and skills versions. +type Entry struct { + Appkit string `json:"appkit"` + Skills string `json:"skills"` +} + +// Manifest is the compatibility manifest: a map of CLI version strings to entries. +type Manifest map[string]Entry + +//go:embed cli-compat.json +var embeddedManifest []byte + +// httpClient is the HTTP client used for manifest fetches. Package-level var +// so tests can replace it. +var httpClient = &http.Client{Timeout: fetchTimeout} + +// FetchManifest fetches the latest manifest from GitHub. +// If the fetch fails, it falls back to the embedded manifest. +func FetchManifest(ctx context.Context) (Manifest, error) { + m, err := fetchRemote(ctx) + if err != nil { + log.Debugf(ctx, "Manifest fetch failed (%v), using embedded fallback", err) + return parseManifest(embeddedManifest) + } + return m, nil +} + +// Resolve returns the manifest entry for the given CLI version. +// +// Resolution order: +// 1. Dev builds (version starts with "0.0.0-dev") use the "next" entry. +// 2. Exact match on CLI version. +// 3. Nearest lower version (semver-sorted). +// 4. If CLI is newer than all entries, use "next". +// 5. If CLI is older than all entries, use "next" (best effort). +func Resolve(m Manifest, cliVersion string) (Entry, error) { + if len(m) == 0 { + return Entry{}, fmt.Errorf("empty compatibility manifest") + } + + next, ok := m[nextKey] + if !ok { + return Entry{}, fmt.Errorf("compatibility manifest missing %q key", nextKey) + } + + // Dev builds always use "next". + if strings.HasPrefix(cliVersion, "0.0.0-dev") { + return next, nil + } + + // Exact match. + if entry, ok := m[cliVersion]; ok { + return entry, nil + } + + // Collect and sort versioned keys (exclude "next"). + var versions []string + for k := range m { + if k != nextKey { + versions = append(versions, k) + } + } + + // Sort descending by semver. The semver package requires a "v" prefix. + sort.Slice(versions, func(i, j int) bool { + return semver.Compare("v"+versions[i], "v"+versions[j]) > 0 + }) + + // Find the nearest lower version. + vCLI := "v" + cliVersion + for _, v := range versions { + if semver.Compare("v"+v, vCLI) <= 0 { + return m[v], nil + } + } + + // CLI is older than all entries — best effort: use "next". + return next, nil +} + +func fetchRemote(ctx context.Context) (Manifest, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, manifestURL, nil) + if err != nil { + return nil, err + } + + resp, err := httpClient.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("HTTP %d fetching manifest", resp.StatusCode) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + + return parseManifest(body) +} + +func parseManifest(data []byte) (Manifest, error) { + var m Manifest + if err := json.Unmarshal(data, &m); err != nil { + return nil, fmt.Errorf("invalid manifest JSON: %w", err) + } + if len(m) == 0 { + return nil, fmt.Errorf("empty compatibility manifest") + } + if _, ok := m[nextKey]; !ok { + return nil, fmt.Errorf("compatibility manifest missing %q key", nextKey) + } + return m, nil +} diff --git a/libs/apps/compat/compat_test.go b/libs/apps/compat/compat_test.go new file mode 100644 index 0000000000..c7ed3df907 --- /dev/null +++ b/libs/apps/compat/compat_test.go @@ -0,0 +1,166 @@ +package compat + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func testManifest() Manifest { + return Manifest{ + "next": {Appkit: "0.27.0", Skills: "0.1.5"}, + "0.296.0": {Appkit: "0.27.0", Skills: "0.1.5"}, + "0.295.0": {Appkit: "0.27.0", Skills: "0.1.5"}, + "0.290.0": {Appkit: "0.24.0", Skills: "0.1.5"}, + "0.288.0": {Appkit: "0.24.0", Skills: "0.1.4"}, + } +} + +func TestResolve_ExactMatch(t *testing.T) { + m := testManifest() + entry, err := Resolve(m, "0.296.0") + require.NoError(t, err) + assert.Equal(t, "0.27.0", entry.Appkit) + assert.Equal(t, "0.1.5", entry.Skills) +} + +func TestResolve_NearestLower(t *testing.T) { + m := testManifest() + // 0.293.0 is between 0.290.0 and 0.295.0 → should use 0.290.0's entry + entry, err := Resolve(m, "0.293.0") + require.NoError(t, err) + assert.Equal(t, "0.24.0", entry.Appkit) + assert.Equal(t, "0.1.5", entry.Skills) +} + +func TestResolve_NewerThanAll(t *testing.T) { + m := testManifest() + entry, err := Resolve(m, "0.300.0") + require.NoError(t, err) + assert.Equal(t, "0.27.0", entry.Appkit) + assert.Equal(t, "0.1.5", entry.Skills) +} + +func TestResolve_DevBuild(t *testing.T) { + m := testManifest() + entry, err := Resolve(m, "0.0.0-dev+abc123def") + require.NoError(t, err) + assert.Equal(t, "0.27.0", entry.Appkit) + assert.Equal(t, "0.1.5", entry.Skills) +} + +func TestResolve_OlderThanAll(t *testing.T) { + m := testManifest() + entry, err := Resolve(m, "0.280.0") + require.NoError(t, err) + // Falls back to "next" (best effort) + assert.Equal(t, "0.27.0", entry.Appkit) +} + +func TestResolve_OnlyNextKey(t *testing.T) { + m := Manifest{ + "next": {Appkit: "0.27.0", Skills: "0.1.5"}, + } + entry, err := Resolve(m, "0.296.0") + require.NoError(t, err) + assert.Equal(t, "0.27.0", entry.Appkit) +} + +func TestResolve_LowestEntryExactMatch(t *testing.T) { + m := testManifest() + entry, err := Resolve(m, "0.288.0") + require.NoError(t, err) + assert.Equal(t, "0.24.0", entry.Appkit) + assert.Equal(t, "0.1.4", entry.Skills) +} + +func TestResolve_EmptyManifest(t *testing.T) { + m := Manifest{} + _, err := Resolve(m, "0.296.0") + assert.Error(t, err) + assert.Contains(t, err.Error(), "empty compatibility manifest") +} + +func TestResolve_MissingNextKey(t *testing.T) { + m := Manifest{ + "0.296.0": {Appkit: "0.27.0", Skills: "0.1.5"}, + } + _, err := Resolve(m, "0.296.0") + assert.Error(t, err) + assert.Contains(t, err.Error(), `missing "next" key`) +} + +func TestFetchManifest_RemoteSuccess(t *testing.T) { + m := testManifest() + body, _ := json.Marshal(m) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(body) + })) + defer srv.Close() + + // Override the manifest URL and HTTP client for this test. + origURL := manifestURL + origClient := httpClient + defer func() { + // Restore. We can't assign to const so we test via fetchRemote indirectly. + httpClient = origClient + }() + httpClient = srv.Client() + + // We need to test fetchRemote directly since manifestURL is a const. + // Instead, test the full FetchManifest with embedded fallback. + result, err := FetchManifest(context.Background()) + require.NoError(t, err) + // Should get a valid manifest from the embedded fallback (since we can't override the const URL). + assert.NotNil(t, result) + assert.Contains(t, result, "next") + _ = origURL +} + +func TestFetchManifest_FallbackToEmbedded(t *testing.T) { + // Create a server that always returns 500. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + // FetchManifest with the real URL will fail (or succeed if GitHub is up), + // but embedded fallback always works. + result, err := FetchManifest(context.Background()) + require.NoError(t, err) + assert.NotNil(t, result) + assert.Contains(t, result, "next") +} + +func TestParseManifest_Valid(t *testing.T) { + data := `{"next":{"appkit":"0.27.0","skills":"0.1.5"},"0.296.0":{"appkit":"0.27.0","skills":"0.1.5"}}` + m, err := parseManifest([]byte(data)) + require.NoError(t, err) + assert.Equal(t, "0.27.0", m["next"].Appkit) + assert.Equal(t, "0.27.0", m["0.296.0"].Appkit) +} + +func TestParseManifest_InvalidJSON(t *testing.T) { + _, err := parseManifest([]byte("not json")) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid manifest JSON") +} + +func TestParseManifest_MissingNext(t *testing.T) { + data := `{"0.296.0":{"appkit":"0.27.0","skills":"0.1.5"}}` + _, err := parseManifest([]byte(data)) + assert.Error(t, err) + assert.Contains(t, err.Error(), `missing "next" key`) +} + +func TestParseManifest_Empty(t *testing.T) { + _, err := parseManifest([]byte("{}")) + assert.Error(t, err) + assert.Contains(t, err.Error(), "empty compatibility manifest") +} From 18606dda28b49816a918cd32ac4072d9e596faea Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Wed, 29 Apr 2026 16:57:21 +0200 Subject: [PATCH 02/10] fix: bound manifest response size and fix fetch tests - Add io.LimitReader (1MB cap) to fetchRemote to guard against corrupted or oversized HTTP responses. - Rewrite TestFetchManifest_RemoteSuccess and TestFetchManifest_FallbackToEmbedded using a roundTripFunc transport so requests actually reach the httptest server. Previously, manifestURL being a const meant the test server was never called and both tests passed only via the embedded fallback. Co-authored-by: Isaac --- libs/apps/compat/compat.go | 3 +- libs/apps/compat/compat_test.go | 62 ++++++++++++++++++++++----------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/libs/apps/compat/compat.go b/libs/apps/compat/compat.go index 19b663cbdd..9403767295 100644 --- a/libs/apps/compat/compat.go +++ b/libs/apps/compat/compat.go @@ -122,7 +122,8 @@ func fetchRemote(ctx context.Context) (Manifest, error) { return nil, fmt.Errorf("HTTP %d fetching manifest", resp.StatusCode) } - body, err := io.ReadAll(resp.Body) + // Cap response size to guard against corrupted or malicious responses. + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) if err != nil { return nil, err } diff --git a/libs/apps/compat/compat_test.go b/libs/apps/compat/compat_test.go index c7ed3df907..7e617c7700 100644 --- a/libs/apps/compat/compat_test.go +++ b/libs/apps/compat/compat_test.go @@ -5,12 +5,36 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "net/url" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// roundTripFunc adapts a function into an http.RoundTripper. +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} + +// redirectToServer returns an http.Client whose transport rewrites every +// request URL to point at srv, keeping manifestURL as a const. +func redirectToServer(t *testing.T, srv *httptest.Server) { + t.Helper() + orig := httpClient + httpClient = &http.Client{ + Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + target, _ := url.Parse(srv.URL) + req.URL.Scheme = target.Scheme + req.URL.Host = target.Host + return http.DefaultTransport.RoundTrip(req) + }), + } + t.Cleanup(func() { httpClient = orig }) +} + func testManifest() Manifest { return Manifest{ "next": {Appkit: "0.27.0", Skills: "0.1.5"}, @@ -96,46 +120,42 @@ func TestResolve_MissingNextKey(t *testing.T) { } func TestFetchManifest_RemoteSuccess(t *testing.T) { - m := testManifest() - body, _ := json.Marshal(m) + want := Manifest{ + "next": {Appkit: "0.99.0", Skills: "0.9.9"}, + "0.296.0": {Appkit: "0.99.0", Skills: "0.9.9"}, + } + body, _ := json.Marshal(want) + var called bool srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true w.Write(body) })) defer srv.Close() + redirectToServer(t, srv) - // Override the manifest URL and HTTP client for this test. - origURL := manifestURL - origClient := httpClient - defer func() { - // Restore. We can't assign to const so we test via fetchRemote indirectly. - httpClient = origClient - }() - httpClient = srv.Client() - - // We need to test fetchRemote directly since manifestURL is a const. - // Instead, test the full FetchManifest with embedded fallback. result, err := FetchManifest(context.Background()) require.NoError(t, err) - // Should get a valid manifest from the embedded fallback (since we can't override the const URL). - assert.NotNil(t, result) - assert.Contains(t, result, "next") - _ = origURL + assert.True(t, called, "test server should have been called") + // Verify we got the remote manifest, not the embedded one. + assert.Equal(t, "0.99.0", result["next"].Appkit) } func TestFetchManifest_FallbackToEmbedded(t *testing.T) { - // Create a server that always returns 500. + var called bool srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true w.WriteHeader(http.StatusInternalServerError) })) defer srv.Close() + redirectToServer(t, srv) - // FetchManifest with the real URL will fail (or succeed if GitHub is up), - // but embedded fallback always works. result, err := FetchManifest(context.Background()) require.NoError(t, err) - assert.NotNil(t, result) + assert.True(t, called, "test server should have been called") + // Should fall back to embedded manifest. assert.Contains(t, result, "next") + assert.Equal(t, "0.27.0", result["next"].Appkit) } func TestParseManifest_Valid(t *testing.T) { From 4b011b4b98e253d7231fd253055a19c74e59c4f9 Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Thu, 30 Apr 2026 11:35:29 +0200 Subject: [PATCH 03/10] fix: address ACE review findings for compat manifest - Fix Resolve docstring to match actual nearest-lower behavior - Strengthen TestResolve_NewerThanAll with distinct next values - Fix import ordering in init.go - Use manifest resolution in manifest.go (was hardcoded) - Validate semver keys in parseManifest - Reduce manifest fetch timeout from 5s to 3s - Fix redirectToServer doc comment - Add missing Skills assertions in tests - Add test for invalid JSON fallback from remote Co-authored-by: Isaac --- cmd/apps/init.go | 2 +- cmd/apps/manifest.go | 8 +++++++- libs/apps/compat/compat.go | 13 +++++++++---- libs/apps/compat/compat_test.go | 27 ++++++++++++++++++++++++--- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/cmd/apps/init.go b/cmd/apps/init.go index bd38541e29..ab9d6c5177 100644 --- a/cmd/apps/init.go +++ b/cmd/apps/init.go @@ -18,8 +18,8 @@ import ( "github.com/charmbracelet/huh" "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/experimental/aitools/lib/agents" - "github.com/databricks/cli/internal/build" "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/apps/compat" "github.com/databricks/cli/libs/apps/generator" "github.com/databricks/cli/libs/apps/initializer" diff --git a/cmd/apps/manifest.go b/cmd/apps/manifest.go index 38df201acc..8d544f0347 100644 --- a/cmd/apps/manifest.go +++ b/cmd/apps/manifest.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/libs/apps/manifest" "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" "github.com/spf13/cobra" ) @@ -27,7 +28,12 @@ func runManifestOnly(ctx context.Context, templatePath, branch, version string) case version != "": gitRef = normalizeVersion(version) default: - gitRef = appkitDefaultVersion + if entry, err := resolveFromManifest(ctx); err != nil { + log.Warnf(ctx, "Manifest resolution failed (%v), using hardcoded default %s", err, appkitDefaultVersion) + gitRef = appkitDefaultVersion + } else { + gitRef = normalizeVersion(entry.Appkit) + } } templateSrc = appkitRepoURL } diff --git a/libs/apps/compat/compat.go b/libs/apps/compat/compat.go index 9403767295..bb7f3015ee 100644 --- a/libs/apps/compat/compat.go +++ b/libs/apps/compat/compat.go @@ -20,7 +20,7 @@ const ( manifestURL = "https://raw.githubusercontent.com/databricks/appkit/main/cli-compat.json" // fetchTimeout is the HTTP timeout for fetching the manifest at runtime. - fetchTimeout = 5 * time.Second + fetchTimeout = 3 * time.Second // nextKey is the special manifest key for CLI versions newer than any entry. nextKey = "next" @@ -58,9 +58,9 @@ func FetchManifest(ctx context.Context) (Manifest, error) { // Resolution order: // 1. Dev builds (version starts with "0.0.0-dev") use the "next" entry. // 2. Exact match on CLI version. -// 3. Nearest lower version (semver-sorted). -// 4. If CLI is newer than all entries, use "next". -// 5. If CLI is older than all entries, use "next" (best effort). +// 3. Nearest lower version (semver-sorted). This also handles CLI versions +// newer than all entries, returning the highest known entry. +// 4. If CLI is older than all entries, use "next" (best effort). func Resolve(m Manifest, cliVersion string) (Entry, error) { if len(m) == 0 { return Entry{}, fmt.Errorf("empty compatibility manifest") @@ -142,5 +142,10 @@ func parseManifest(data []byte) (Manifest, error) { if _, ok := m[nextKey]; !ok { return nil, fmt.Errorf("compatibility manifest missing %q key", nextKey) } + for k := range m { + if k != nextKey && !semver.IsValid("v"+k) { + return nil, fmt.Errorf("invalid semver key %q in compatibility manifest", k) + } + } return m, nil } diff --git a/libs/apps/compat/compat_test.go b/libs/apps/compat/compat_test.go index 7e617c7700..00b6cdab5c 100644 --- a/libs/apps/compat/compat_test.go +++ b/libs/apps/compat/compat_test.go @@ -19,8 +19,8 @@ func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) } -// redirectToServer returns an http.Client whose transport rewrites every -// request URL to point at srv, keeping manifestURL as a const. +// redirectToServer replaces the package-level httpClient with one whose +// transport rewrites every request URL to point at srv. func redirectToServer(t *testing.T, srv *httptest.Server) { t.Helper() orig := httpClient @@ -63,9 +63,14 @@ func TestResolve_NearestLower(t *testing.T) { } func TestResolve_NewerThanAll(t *testing.T) { - m := testManifest() + m := Manifest{ + "next": {Appkit: "0.99.0", Skills: "0.9.9"}, + "0.296.0": {Appkit: "0.27.0", Skills: "0.1.5"}, + "0.290.0": {Appkit: "0.24.0", Skills: "0.1.5"}, + } entry, err := Resolve(m, "0.300.0") require.NoError(t, err) + // Nearest-lower returns the highest versioned entry, not "next". assert.Equal(t, "0.27.0", entry.Appkit) assert.Equal(t, "0.1.5", entry.Skills) } @@ -84,6 +89,7 @@ func TestResolve_OlderThanAll(t *testing.T) { require.NoError(t, err) // Falls back to "next" (best effort) assert.Equal(t, "0.27.0", entry.Appkit) + assert.Equal(t, "0.1.5", entry.Skills) } func TestResolve_OnlyNextKey(t *testing.T) { @@ -93,6 +99,7 @@ func TestResolve_OnlyNextKey(t *testing.T) { entry, err := Resolve(m, "0.296.0") require.NoError(t, err) assert.Equal(t, "0.27.0", entry.Appkit) + assert.Equal(t, "0.1.5", entry.Skills) } func TestResolve_LowestEntryExactMatch(t *testing.T) { @@ -158,6 +165,20 @@ func TestFetchManifest_FallbackToEmbedded(t *testing.T) { assert.Equal(t, "0.27.0", result["next"].Appkit) } +func TestFetchManifest_RemoteReturnsInvalidJSON(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("not json at all")) + })) + defer srv.Close() + redirectToServer(t, srv) + + result, err := FetchManifest(context.Background()) + require.NoError(t, err) + // Should fall back to embedded manifest. + assert.Contains(t, result, "next") + assert.Equal(t, "0.27.0", result["next"].Appkit) +} + func TestParseManifest_Valid(t *testing.T) { data := `{"next":{"appkit":"0.27.0","skills":"0.1.5"},"0.296.0":{"appkit":"0.27.0","skills":"0.1.5"}}` m, err := parseManifest([]byte(data)) From 6af209dc16b7855c1b4bb830f1cb12aa2b053175 Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Thu, 30 Apr 2026 14:36:19 +0200 Subject: [PATCH 04/10] feat: add 1h local cache, retry, and build-time fetch for compat manifest - Cache fetched manifest locally for 1h using libs/cache - Add retry logic (2 retries, 300ms backoff) for runtime fetches - Fetch latest manifest during build (Makefile + goreleaser) - Replace checked-in manifest with {} placeholder - Remove hardcoded appkitDefaultVersion fallback; manifest is sole source of truth - Print resolved template and skills versions to user during apps init - Remove version from skills post-install message (shown upfront instead) Co-authored-by: Isaac --- .goreleaser.yaml | 1 + Makefile | 21 +++-- cmd/apps/init.go | 20 ++-- cmd/apps/init_test.go | 2 +- cmd/apps/manifest.go | 10 +- .../aitools/lib/installer/installer.go | 3 +- .../aitools/lib/installer/installer_test.go | 17 ++-- libs/apps/compat/cli-compat.json | 8 +- libs/apps/compat/compat.go | 44 ++++++++- libs/apps/compat/compat_test.go | 91 ++++++++++++++++--- 10 files changed, 157 insertions(+), 60 deletions(-) diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 8766664f14..200803a534 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -3,6 +3,7 @@ version: 2 before: hooks: - go mod download + - make fetch-compat-manifest builds: - env: diff --git a/Makefile b/Makefile index 3896caa775..333e76853b 100644 --- a/Makefile +++ b/Makefile @@ -134,15 +134,24 @@ showcover: acc-showcover: go tool cover -html=coverage-acceptance.txt +COMPAT_MANIFEST_URL := https://raw.githubusercontent.com/databricks/appkit/main/cli-compat.json +COMPAT_MANIFEST_PATH := libs/apps/compat/cli-compat.json + .PHONY: fetch-compat-manifest fetch-compat-manifest: - @curl -sfL https://raw.githubusercontent.com/databricks/appkit/main/cli-compat.json \ - -o libs/apps/compat/cli-compat.json \ - && echo "Fetched latest cli-compat.json" \ - || echo "Warning: failed to fetch cli-compat.json, using existing copy" + @echo "Fetching compatibility manifest..." + @for attempt in 1 2 3; do \ + if wget -qO $(COMPAT_MANIFEST_PATH) $(COMPAT_MANIFEST_URL); then \ + echo "Fetched $(COMPAT_MANIFEST_PATH)"; \ + exit 0; \ + fi; \ + echo " Attempt $$attempt/3 failed"; \ + [ $$attempt -lt 3 ] && sleep 2; \ + done; \ + echo "Warning: failed to fetch compatibility manifest after 3 attempts; the CLI will resolve versions at runtime" .PHONY: build -build: tidy +build: tidy fetch-compat-manifest go build # builds the binary in a VM environment (such as Parallels Desktop) where your files are mirrored from the host os @@ -151,7 +160,7 @@ build-vm: tidy go build -buildvcs=false .PHONY: snapshot -snapshot: +snapshot: fetch-compat-manifest go build -o .databricks/databricks # Produce release binaries and archives in the dist folder without uploading them anywhere. diff --git a/cmd/apps/init.go b/cmd/apps/init.go index ab9d6c5177..e42b8fc5b4 100644 --- a/cmd/apps/init.go +++ b/cmd/apps/init.go @@ -40,8 +40,7 @@ const ( appkitTemplateDir = "template" appkitDefaultBranch = "main" appkitTemplateTagPfx = "template-v" - appkitDefaultVersion = "template-v0.24.0" - defaultProfile = "DEFAULT" + defaultProfile = "DEFAULT" ) // normalizeVersion converts a version string to the template tag format "template-vX.X.X". @@ -181,7 +180,7 @@ Environment variables: cmd.Flags().StringVar(&templatePath, "template", "", "Template path (local directory or GitHub URL)") cmd.Flags().StringVar(&branch, "branch", "", "Git branch or tag (for GitHub templates, mutually exclusive with --version)") - cmd.Flags().StringVar(&version, "version", "", fmt.Sprintf("AppKit version to use (default: %s, use 'latest' for main branch)", appkitDefaultVersion)) + cmd.Flags().StringVar(&version, "version", "", "AppKit version to use (resolved from compatibility manifest, use 'latest' for main branch)") cmd.Flags().StringVar(&name, "name", "", "Project name (prompts if not provided)") cmd.Flags().StringVar(&warehouseID, "warehouse-id", "", "SQL warehouse ID") _ = cmd.Flags().MarkDeprecated("warehouse-id", "use --set .sql-warehouse.id= instead") @@ -790,14 +789,14 @@ func runCreate(ctx context.Context, opts createOptions) error { gitRef = normalizeVersion(opts.version) default: // Resolve from compatibility manifest (fetch from GitHub, fall back to embedded). - if entry, err := resolveFromManifest(ctx); err != nil { - log.Warnf(ctx, "Manifest resolution failed (%v), using hardcoded default %s", err, appkitDefaultVersion) - gitRef = appkitDefaultVersion - } else { - gitRef = normalizeVersion(entry.Appkit) - resolvedSkillsVersion = entry.Skills - log.Infof(ctx, "Resolved AppKit template version %s (skills %s) from compatibility manifest for CLI %s", entry.Appkit, entry.Skills, build.GetInfo().Version) + entry, err := resolveFromManifest(ctx) + if err != nil { + return fmt.Errorf("could not resolve AppKit template version: %w. Use --version to specify a version manually", err) } + gitRef = normalizeVersion(entry.Appkit) + resolvedSkillsVersion = entry.Skills + cmdio.LogString(ctx, fmt.Sprintf("Using AppKit template version %s", entry.Appkit)) + log.Debugf(ctx, "Resolved skills %s from compatibility manifest for CLI %s", entry.Skills, build.GetInfo().Version) } templateSrc = appkitRepoURL } @@ -1157,6 +1156,7 @@ func runCreate(ctx context.Context, opts createOptions) error { skillsCtx := ctx if resolvedSkillsVersion != "" { skillsCtx = env.Set(ctx, "DATABRICKS_SKILLS_REF", "v"+resolvedSkillsVersion) + cmdio.LogString(ctx, fmt.Sprintf("Using skills version %s", resolvedSkillsVersion)) } // Recommend skills installation if coding agents are detected without skills. diff --git a/cmd/apps/init_test.go b/cmd/apps/init_test.go index d837474e12..6a1333f8b0 100644 --- a/cmd/apps/init_test.go +++ b/cmd/apps/init_test.go @@ -281,7 +281,7 @@ func TestNormalizeVersion(t *testing.T) { {"", ""}, {"main", "main"}, {"feat/something", "feat/something"}, - {appkitDefaultVersion, appkitDefaultVersion}, + {"template-v0.24.0", "template-v0.24.0"}, } for _, tt := range tests { diff --git a/cmd/apps/manifest.go b/cmd/apps/manifest.go index 8d544f0347..68daf63e2d 100644 --- a/cmd/apps/manifest.go +++ b/cmd/apps/manifest.go @@ -10,7 +10,6 @@ import ( "github.com/databricks/cli/libs/apps/manifest" "github.com/databricks/cli/libs/env" - "github.com/databricks/cli/libs/log" "github.com/spf13/cobra" ) @@ -28,12 +27,11 @@ func runManifestOnly(ctx context.Context, templatePath, branch, version string) case version != "": gitRef = normalizeVersion(version) default: - if entry, err := resolveFromManifest(ctx); err != nil { - log.Warnf(ctx, "Manifest resolution failed (%v), using hardcoded default %s", err, appkitDefaultVersion) - gitRef = appkitDefaultVersion - } else { - gitRef = normalizeVersion(entry.Appkit) + entry, err := resolveFromManifest(ctx) + if err != nil { + return fmt.Errorf("could not resolve AppKit template version: %w. Use --version to specify a version manually", err) } + gitRef = normalizeVersion(entry.Appkit) } templateSrc = appkitRepoURL } diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go index 982df0c163..f9d8d64741 100644 --- a/experimental/aitools/lib/installer/installer.go +++ b/experimental/aitools/lib/installer/installer.go @@ -194,12 +194,11 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent return err } - tag := strings.TrimPrefix(ref, "v") noun := "skills" if len(targetSkills) == 1 { noun = "skill" } - cmdio.LogString(ctx, fmt.Sprintf("Installed %d %s (v%s).", len(targetSkills), noun, tag)) + cmdio.LogString(ctx, fmt.Sprintf("Installed %d %s.", len(targetSkills), noun)) return nil } diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go index b769143906..162c7abb8a 100644 --- a/experimental/aitools/lib/installer/installer_test.go +++ b/experimental/aitools/lib/installer/installer_test.go @@ -3,12 +3,10 @@ package installer import ( "bytes" "context" - "fmt" "io/fs" "log/slog" "os" "path/filepath" - "strings" "testing" "github.com/databricks/cli/experimental/aitools/lib/agents" @@ -209,7 +207,7 @@ func TestInstallSkillsForAgentsWritesState(t *testing.T) { assert.Equal(t, "0.1.0", state.Skills["databricks-sql"]) assert.Equal(t, "0.1.0", state.Skills["databricks-jobs"]) - assert.Contains(t, stderr.String(), fmt.Sprintf("Installed 2 skills (%s).", defaultSkillsRepoRef)) + assert.Contains(t, stderr.String(), "Installed 2 skills.") } func TestInstallSkillForSingleWritesState(t *testing.T) { @@ -232,7 +230,7 @@ func TestInstallSkillForSingleWritesState(t *testing.T) { assert.Len(t, state.Skills, 1) assert.Equal(t, "0.1.0", state.Skills["databricks-sql"]) - assert.Contains(t, stderr.String(), fmt.Sprintf("Installed 1 skill (%s).", defaultSkillsRepoRef)) + assert.Contains(t, stderr.String(), "Installed 1 skill.") } func TestInstallSkillsSpecificNotFound(t *testing.T) { @@ -275,7 +273,7 @@ func TestExperimentalSkillsSkippedByDefault(t *testing.T) { assert.Len(t, state.Skills, 2) assert.NotContains(t, state.Skills, "databricks-experimental") - assert.Contains(t, stderr.String(), fmt.Sprintf("Installed 2 skills (%s).", defaultSkillsRepoRef)) + assert.Contains(t, stderr.String(), "Installed 2 skills.") } func TestExperimentalSkillsIncludedWithFlag(t *testing.T) { @@ -305,7 +303,7 @@ func TestExperimentalSkillsIncludedWithFlag(t *testing.T) { assert.Contains(t, state.Skills, "databricks-experimental") assert.True(t, state.IncludeExperimental) - assert.Contains(t, stderr.String(), fmt.Sprintf("Installed 3 skills (%s).", defaultSkillsRepoRef)) + assert.Contains(t, stderr.String(), "Installed 3 skills.") } func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) { @@ -339,7 +337,7 @@ func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) { assert.Len(t, state.Skills, 2) assert.NotContains(t, state.Skills, "databricks-future") - assert.Contains(t, stderr.String(), fmt.Sprintf("Installed 2 skills (%s).", defaultSkillsRepoRef)) + assert.Contains(t, stderr.String(), "Installed 2 skills.") assert.Contains(t, logBuf.String(), "requires CLI version 0.300.0") } @@ -610,8 +608,7 @@ func TestInstallProjectScopeWritesState(t *testing.T) { assert.Equal(t, defaultSkillsRepoRef, state.Release) assert.Len(t, state.Skills, 2) - tag := strings.TrimPrefix(defaultSkillsRepoRef, "v") - assert.Contains(t, stderr.String(), fmt.Sprintf("Installed 2 skills (v%s).", tag)) + assert.Contains(t, stderr.String(), "Installed 2 skills.") } func TestInstallProjectScopeCreatesSymlinks(t *testing.T) { @@ -680,7 +677,7 @@ func TestInstallProjectScopeFiltersIncompatibleAgents(t *testing.T) { require.NoError(t, err) assert.Contains(t, stderr.String(), "Skipped No Project Agent: does not support project-scoped skills.") - assert.Contains(t, stderr.String(), fmt.Sprintf("Installed 2 skills (%s).", defaultSkillsRepoRef)) + assert.Contains(t, stderr.String(), "Installed 2 skills.") } func TestInstallProjectScopeZeroCompatibleAgentsReturnsError(t *testing.T) { diff --git a/libs/apps/compat/cli-compat.json b/libs/apps/compat/cli-compat.json index fd13c5f96b..9e26dfeeb6 100644 --- a/libs/apps/compat/cli-compat.json +++ b/libs/apps/compat/cli-compat.json @@ -1,7 +1 @@ -{ - "next": { "appkit": "0.27.0", "skills": "0.1.5" }, - "0.296.0": { "appkit": "0.27.0", "skills": "0.1.5" }, - "0.295.0": { "appkit": "0.27.0", "skills": "0.1.5" }, - "0.290.0": { "appkit": "0.24.0", "skills": "0.1.5" }, - "0.288.0": { "appkit": "0.24.0", "skills": "0.1.4" } -} +{} \ No newline at end of file diff --git a/libs/apps/compat/compat.go b/libs/apps/compat/compat.go index bb7f3015ee..d44405a04b 100644 --- a/libs/apps/compat/compat.go +++ b/libs/apps/compat/compat.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/databricks/cli/libs/cache" "github.com/databricks/cli/libs/log" "golang.org/x/mod/semver" ) @@ -24,8 +25,18 @@ const ( // nextKey is the special manifest key for CLI versions newer than any entry. nextKey = "next" + + cacheComponent = "compat-manifest" + cacheTTL = 1 * time.Hour + fetchRetries = 2 + fetchRetryBackoff = 300 * time.Millisecond ) +// manifestFingerprint is the cache key for the compatibility manifest. +type manifestFingerprint struct { + URL string `json:"url"` +} + // Entry maps a CLI version to compatible AppKit and skills versions. type Entry struct { Appkit string `json:"appkit"` @@ -42,10 +53,15 @@ var embeddedManifest []byte // so tests can replace it. var httpClient = &http.Client{Timeout: fetchTimeout} -// FetchManifest fetches the latest manifest from GitHub. -// If the fetch fails, it falls back to the embedded manifest. +// FetchManifest returns the compatibility manifest, checking in order: +// 1h local disk cache, remote fetch with retry, embedded fallback. func FetchManifest(ctx context.Context) (Manifest, error) { - m, err := fetchRemote(ctx) + c := cache.NewCache(ctx, cacheComponent, cacheTTL, nil) + fp := manifestFingerprint{URL: manifestURL} + + m, err := cache.GetOrCompute(ctx, c, fp, func(ctx context.Context) (Manifest, error) { + return fetchRemoteWithRetry(ctx) + }) if err != nil { log.Debugf(ctx, "Manifest fetch failed (%v), using embedded fallback", err) return parseManifest(embeddedManifest) @@ -106,7 +122,29 @@ func Resolve(m Manifest, cliVersion string) (Entry, error) { return next, nil } +// fetchRemoteWithRetry wraps fetchRemote with retries. +func fetchRemoteWithRetry(ctx context.Context) (Manifest, error) { + var lastErr error + for attempt := range fetchRetries + 1 { + if attempt > 0 { + log.Debugf(ctx, "Retrying manifest fetch (attempt %d)", attempt+1) + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(fetchRetryBackoff): + } + } + m, err := fetchRemote(ctx) + if err == nil { + return m, nil + } + lastErr = err + } + return nil, lastErr +} + func fetchRemote(ctx context.Context) (Manifest, error) { + log.Debugf(ctx, "Fetching compatibility manifest from %s", manifestURL) req, err := http.NewRequestWithContext(ctx, http.MethodGet, manifestURL, nil) if err != nil { return nil, err diff --git a/libs/apps/compat/compat_test.go b/libs/apps/compat/compat_test.go index 00b6cdab5c..f3b30c9edc 100644 --- a/libs/apps/compat/compat_test.go +++ b/libs/apps/compat/compat_test.go @@ -6,8 +6,10 @@ import ( "net/http" "net/http/httptest" "net/url" + "sync/atomic" "testing" + "github.com/databricks/cli/libs/env" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -35,6 +37,13 @@ func redirectToServer(t *testing.T, srv *httptest.Server) { t.Cleanup(func() { httpClient = orig }) } +// testContext returns a context with an isolated cache directory so tests don't +// share cached manifests. +func testContext(t *testing.T) context.Context { + t.Helper() + return env.Set(t.Context(), "DATABRICKS_CACHE_DIR", t.TempDir()) +} + func testManifest() Manifest { return Manifest{ "next": {Appkit: "0.27.0", Skills: "0.1.5"}, @@ -127,6 +136,7 @@ func TestResolve_MissingNextKey(t *testing.T) { } func TestFetchManifest_RemoteSuccess(t *testing.T) { + ctx := testContext(t) want := Manifest{ "next": {Appkit: "0.99.0", Skills: "0.9.9"}, "0.296.0": {Appkit: "0.99.0", Skills: "0.9.9"}, @@ -141,42 +151,93 @@ func TestFetchManifest_RemoteSuccess(t *testing.T) { defer srv.Close() redirectToServer(t, srv) - result, err := FetchManifest(context.Background()) + result, err := FetchManifest(ctx) require.NoError(t, err) assert.True(t, called, "test server should have been called") - // Verify we got the remote manifest, not the embedded one. assert.Equal(t, "0.99.0", result["next"].Appkit) } -func TestFetchManifest_FallbackToEmbedded(t *testing.T) { - var called bool +// With {} as the embedded manifest, a remote failure means both remote and +// embedded fail → FetchManifest returns an error. +func TestFetchManifest_RemoteFailReturnsError(t *testing.T) { + ctx := testContext(t) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - called = true w.WriteHeader(http.StatusInternalServerError) })) defer srv.Close() redirectToServer(t, srv) - result, err := FetchManifest(context.Background()) - require.NoError(t, err) - assert.True(t, called, "test server should have been called") - // Should fall back to embedded manifest. - assert.Contains(t, result, "next") - assert.Equal(t, "0.27.0", result["next"].Appkit) + _, err := FetchManifest(ctx) + assert.Error(t, err) + assert.Contains(t, err.Error(), "empty compatibility manifest") } func TestFetchManifest_RemoteReturnsInvalidJSON(t *testing.T) { + ctx := testContext(t) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("not json at all")) })) defer srv.Close() redirectToServer(t, srv) - result, err := FetchManifest(context.Background()) + _, err := FetchManifest(ctx) + assert.Error(t, err) + assert.Contains(t, err.Error(), "empty compatibility manifest") +} + +func TestFetchManifest_CacheHit(t *testing.T) { + ctx := testContext(t) + want := Manifest{ + "next": {Appkit: "0.99.0", Skills: "0.9.9"}, + "0.296.0": {Appkit: "0.99.0", Skills: "0.9.9"}, + } + body, _ := json.Marshal(want) + + var callCount atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount.Add(1) + w.Write(body) + })) + defer srv.Close() + redirectToServer(t, srv) + + // First call: populates cache. + result1, err := FetchManifest(ctx) + require.NoError(t, err) + assert.Equal(t, "0.99.0", result1["next"].Appkit) + + // Second call: should come from cache, not hitting the server again. + result2, err := FetchManifest(ctx) + require.NoError(t, err) + assert.Equal(t, "0.99.0", result2["next"].Appkit) + + assert.Equal(t, int32(1), callCount.Load(), "server should only be called once; second call should be a cache hit") +} + +func TestFetchManifest_RetryOnTransientError(t *testing.T) { + ctx := testContext(t) + want := Manifest{ + "next": {Appkit: "0.99.0", Skills: "0.9.9"}, + "0.296.0": {Appkit: "0.99.0", Skills: "0.9.9"}, + } + body, _ := json.Marshal(want) + + var callCount atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n := callCount.Add(1) + if n == 1 { + w.WriteHeader(http.StatusInternalServerError) + return + } + w.Write(body) + })) + defer srv.Close() + redirectToServer(t, srv) + + result, err := FetchManifest(ctx) require.NoError(t, err) - // Should fall back to embedded manifest. - assert.Contains(t, result, "next") - assert.Equal(t, "0.27.0", result["next"].Appkit) + assert.Equal(t, "0.99.0", result["next"].Appkit) + assert.Equal(t, int32(2), callCount.Load(), "should have retried after first failure") } func TestParseManifest_Valid(t *testing.T) { From bdf28f36b9cf1102d6fdb07d767d9860c4289af8 Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Thu, 30 Apr 2026 17:10:11 +0200 Subject: [PATCH 05/10] refactor: embed only resolved dep versions, move compat to depversions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move libs/apps/compat/ → libs/depversions/ (cross-cutting, not apps-only) - Rename Entry.Appkit → Entry.AppKit - Embed resolved entry in internal/build/dep_versions.json instead of full manifest - Add build-time pin tool (internal/pindepversions) for goreleaser - Add ResolveAppKitVersion/ResolveAgentSkillsVersion convenience helpers - Remove cli-compat.json and fetch-compat-manifest Makefile target - Resolve() returns lowest entry (not "next") for old CLI versions Co-authored-by: Isaac --- .goreleaser.yaml | 2 +- Makefile | 20 +--- cmd/apps/init.go | 27 ++--- cmd/apps/manifest.go | 5 +- internal/build/dep_versions.go | 28 +++++ .../build/dep_versions.json | 0 internal/pindepversions/main.go | 46 ++++++++ libs/{apps/compat => depversions}/compat.go | 71 +++++++++--- .../compat => depversions}/compat_test.go | 103 ++++++++++++------ 9 files changed, 210 insertions(+), 92 deletions(-) create mode 100644 internal/build/dep_versions.go rename libs/apps/compat/cli-compat.json => internal/build/dep_versions.json (100%) create mode 100644 internal/pindepversions/main.go rename libs/{apps/compat => depversions}/compat.go (70%) rename libs/{apps/compat => depversions}/compat_test.go (70%) diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 200803a534..91ad43f8a0 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -3,7 +3,7 @@ version: 2 before: hooks: - go mod download - - make fetch-compat-manifest + - go run ./internal/pindepversions --version {{ .Version }} builds: - env: diff --git a/Makefile b/Makefile index 333e76853b..75e7d0860d 100644 --- a/Makefile +++ b/Makefile @@ -134,24 +134,8 @@ showcover: acc-showcover: go tool cover -html=coverage-acceptance.txt -COMPAT_MANIFEST_URL := https://raw.githubusercontent.com/databricks/appkit/main/cli-compat.json -COMPAT_MANIFEST_PATH := libs/apps/compat/cli-compat.json - -.PHONY: fetch-compat-manifest -fetch-compat-manifest: - @echo "Fetching compatibility manifest..." - @for attempt in 1 2 3; do \ - if wget -qO $(COMPAT_MANIFEST_PATH) $(COMPAT_MANIFEST_URL); then \ - echo "Fetched $(COMPAT_MANIFEST_PATH)"; \ - exit 0; \ - fi; \ - echo " Attempt $$attempt/3 failed"; \ - [ $$attempt -lt 3 ] && sleep 2; \ - done; \ - echo "Warning: failed to fetch compatibility manifest after 3 attempts; the CLI will resolve versions at runtime" - .PHONY: build -build: tidy fetch-compat-manifest +build: tidy go build # builds the binary in a VM environment (such as Parallels Desktop) where your files are mirrored from the host os @@ -160,7 +144,7 @@ build-vm: tidy go build -buildvcs=false .PHONY: snapshot -snapshot: fetch-compat-manifest +snapshot: go build -o .databricks/databricks # Produce release binaries and archives in the dist folder without uploading them anywhere. diff --git a/cmd/apps/init.go b/cmd/apps/init.go index e42b8fc5b4..754c3fe985 100644 --- a/cmd/apps/init.go +++ b/cmd/apps/init.go @@ -19,8 +19,7 @@ import ( "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/experimental/aitools/lib/agents" "github.com/databricks/cli/experimental/aitools/lib/installer" - "github.com/databricks/cli/internal/build" - "github.com/databricks/cli/libs/apps/compat" + "github.com/databricks/cli/libs/depversions" "github.com/databricks/cli/libs/apps/generator" "github.com/databricks/cli/libs/apps/initializer" "github.com/databricks/cli/libs/apps/manifest" @@ -65,16 +64,6 @@ func normalizeVersion(version string) string { return version } -// resolveFromManifest fetches the compatibility manifest and resolves the -// entry for the current CLI version. -func resolveFromManifest(ctx context.Context) (compat.Entry, error) { - m, err := compat.FetchManifest(ctx) - if err != nil { - return compat.Entry{}, err - } - return compat.Resolve(m, build.GetInfo().Version) -} - func newInitCmd() *cobra.Command { var ( templatePath string @@ -788,15 +777,17 @@ func runCreate(ctx context.Context, opts createOptions) error { case opts.version != "": gitRef = normalizeVersion(opts.version) default: - // Resolve from compatibility manifest (fetch from GitHub, fall back to embedded). - entry, err := resolveFromManifest(ctx) + appkitVersion, err := depversions.ResolveAppKitVersion(ctx) if err != nil { return fmt.Errorf("could not resolve AppKit template version: %w. Use --version to specify a version manually", err) } - gitRef = normalizeVersion(entry.Appkit) - resolvedSkillsVersion = entry.Skills - cmdio.LogString(ctx, fmt.Sprintf("Using AppKit template version %s", entry.Appkit)) - log.Debugf(ctx, "Resolved skills %s from compatibility manifest for CLI %s", entry.Skills, build.GetInfo().Version) + gitRef = normalizeVersion(appkitVersion) + cmdio.LogString(ctx, fmt.Sprintf("Using AppKit template version %s", appkitVersion)) + + skillsVersion, err := depversions.ResolveAgentSkillsVersion(ctx) + if err == nil { + resolvedSkillsVersion = skillsVersion + } } templateSrc = appkitRepoURL } diff --git a/cmd/apps/manifest.go b/cmd/apps/manifest.go index 68daf63e2d..ff200e6998 100644 --- a/cmd/apps/manifest.go +++ b/cmd/apps/manifest.go @@ -9,6 +9,7 @@ import ( "path/filepath" "github.com/databricks/cli/libs/apps/manifest" + "github.com/databricks/cli/libs/depversions" "github.com/databricks/cli/libs/env" "github.com/spf13/cobra" ) @@ -27,11 +28,11 @@ func runManifestOnly(ctx context.Context, templatePath, branch, version string) case version != "": gitRef = normalizeVersion(version) default: - entry, err := resolveFromManifest(ctx) + appkitVersion, err := depversions.ResolveAppKitVersion(ctx) if err != nil { return fmt.Errorf("could not resolve AppKit template version: %w. Use --version to specify a version manually", err) } - gitRef = normalizeVersion(entry.Appkit) + gitRef = normalizeVersion(appkitVersion) } templateSrc = appkitRepoURL } diff --git a/internal/build/dep_versions.go b/internal/build/dep_versions.go new file mode 100644 index 0000000000..c4801c2361 --- /dev/null +++ b/internal/build/dep_versions.go @@ -0,0 +1,28 @@ +package build + +import ( + _ "embed" + "encoding/json" + "sync" +) + +//go:embed dep_versions.json +var depVersionsJSON []byte + +// DepVersions holds build-time resolved dependency versions. +type DepVersions struct { + AppKit string `json:"appkit"` + Skills string `json:"skills"` +} + +var depVersions = sync.OnceValue(func() DepVersions { + var dv DepVersions + _ = json.Unmarshal(depVersionsJSON, &dv) + return dv +}) + +// GetDepVersions returns the build-time resolved dependency versions. +// Returns zero-value DepVersions if not set (dev builds). +func GetDepVersions() DepVersions { + return depVersions() +} diff --git a/libs/apps/compat/cli-compat.json b/internal/build/dep_versions.json similarity index 100% rename from libs/apps/compat/cli-compat.json rename to internal/build/dep_versions.json diff --git a/internal/pindepversions/main.go b/internal/pindepversions/main.go new file mode 100644 index 0000000000..03e3602937 --- /dev/null +++ b/internal/pindepversions/main.go @@ -0,0 +1,46 @@ +// Command pindepversions fetches the compatibility manifest and writes +// the resolved entry for a given CLI version. Used at release build time +// to embed version-specific dep versions into the binary. +package main + +import ( + "context" + "encoding/json" + "flag" + "fmt" + "os" + + "github.com/databricks/cli/libs/depversions" +) + +const defaultOutput = "internal/build/dep_versions.json" + +func main() { + version := flag.String("version", "", "CLI version to resolve (required)") + output := flag.String("o", defaultOutput, "output file path") + flag.Parse() + + if *version == "" { + fmt.Fprintln(os.Stderr, "error: --version is required") + os.Exit(1) + } + + m, err := depversions.FetchManifest(context.Background()) + if err != nil { + fmt.Fprintf(os.Stderr, "warning: could not fetch manifest: %v\n", err) + return + } + + entry, err := depversions.Resolve(m, *version) + if err != nil { + fmt.Fprintf(os.Stderr, "warning: could not resolve version %s: %v\n", *version, err) + return + } + + data, _ := json.Marshal(entry) + if err := os.WriteFile(*output, data, 0o644); err != nil { + fmt.Fprintf(os.Stderr, "error: write %s: %v\n", *output, err) + os.Exit(1) + } + fmt.Fprintf(os.Stderr, "Pinned for CLI %s: appkit=%s skills=%s\n", *version, entry.AppKit, entry.Skills) +} diff --git a/libs/apps/compat/compat.go b/libs/depversions/compat.go similarity index 70% rename from libs/apps/compat/compat.go rename to libs/depversions/compat.go index d44405a04b..d0df1ae496 100644 --- a/libs/apps/compat/compat.go +++ b/libs/depversions/compat.go @@ -1,8 +1,7 @@ -package compat +package depversions import ( "context" - _ "embed" "encoding/json" "fmt" "io" @@ -11,6 +10,7 @@ import ( "strings" "time" + "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/cache" "github.com/databricks/cli/libs/log" "golang.org/x/mod/semver" @@ -39,34 +39,25 @@ type manifestFingerprint struct { // Entry maps a CLI version to compatible AppKit and skills versions. type Entry struct { - Appkit string `json:"appkit"` + AppKit string `json:"appkit"` Skills string `json:"skills"` } // Manifest is the compatibility manifest: a map of CLI version strings to entries. type Manifest map[string]Entry -//go:embed cli-compat.json -var embeddedManifest []byte - // httpClient is the HTTP client used for manifest fetches. Package-level var // so tests can replace it. var httpClient = &http.Client{Timeout: fetchTimeout} // FetchManifest returns the compatibility manifest, checking in order: -// 1h local disk cache, remote fetch with retry, embedded fallback. +// 1h local disk cache, then remote fetch with retry. func FetchManifest(ctx context.Context) (Manifest, error) { c := cache.NewCache(ctx, cacheComponent, cacheTTL, nil) fp := manifestFingerprint{URL: manifestURL} - - m, err := cache.GetOrCompute(ctx, c, fp, func(ctx context.Context) (Manifest, error) { + return cache.GetOrCompute(ctx, c, fp, func(ctx context.Context) (Manifest, error) { return fetchRemoteWithRetry(ctx) }) - if err != nil { - log.Debugf(ctx, "Manifest fetch failed (%v), using embedded fallback", err) - return parseManifest(embeddedManifest) - } - return m, nil } // Resolve returns the manifest entry for the given CLI version. @@ -76,7 +67,7 @@ func FetchManifest(ctx context.Context) (Manifest, error) { // 2. Exact match on CLI version. // 3. Nearest lower version (semver-sorted). This also handles CLI versions // newer than all entries, returning the highest known entry. -// 4. If CLI is older than all entries, use "next" (best effort). +// 4. If CLI is older than all entries, use the lowest (oldest) entry. func Resolve(m Manifest, cliVersion string) (Entry, error) { if len(m) == 0 { return Entry{}, fmt.Errorf("empty compatibility manifest") @@ -118,8 +109,54 @@ func Resolve(m Manifest, cliVersion string) (Entry, error) { } } - // CLI is older than all entries — best effort: use "next". - return next, nil + // CLI is older than all entries — use the lowest (oldest) entry as closest match. + // If there are no versioned entries (only "next"), fall back to "next". + if len(versions) == 0 { + return next, nil + } + return m[versions[len(versions)-1]], nil +} + +// resolveEntry fetches the manifest, resolves for the given CLI version, +// and falls back to the build-time dep versions on failure. +func resolveEntry(ctx context.Context, cliVersion string) (Entry, error) { + m, fetchErr := FetchManifest(ctx) + if fetchErr == nil { + entry, err := Resolve(m, cliVersion) + if err == nil { + return entry, nil + } + log.Debugf(ctx, "Resolve failed (%v), trying build-time fallback", err) + } + + dv := build.GetDepVersions() + if dv.AppKit != "" { + log.Debugf(ctx, "Using build-time dep versions: appkit=%s skills=%s", dv.AppKit, dv.Skills) + return Entry{AppKit: dv.AppKit, Skills: dv.Skills}, nil + } + + if fetchErr != nil { + return Entry{}, fmt.Errorf("manifest fetch failed and no build-time versions available: %w", fetchErr) + } + return Entry{}, fmt.Errorf("no compatible versions available") +} + +// ResolveAppKitVersion resolves the AppKit template version for the current CLI. +func ResolveAppKitVersion(ctx context.Context) (string, error) { + entry, err := resolveEntry(ctx, build.GetInfo().Version) + if err != nil { + return "", err + } + return entry.AppKit, nil +} + +// ResolveAgentSkillsVersion resolves the Agent Skills version for the current CLI. +func ResolveAgentSkillsVersion(ctx context.Context) (string, error) { + entry, err := resolveEntry(ctx, build.GetInfo().Version) + if err != nil { + return "", err + } + return entry.Skills, nil } // fetchRemoteWithRetry wraps fetchRemote with retries. diff --git a/libs/apps/compat/compat_test.go b/libs/depversions/compat_test.go similarity index 70% rename from libs/apps/compat/compat_test.go rename to libs/depversions/compat_test.go index f3b30c9edc..e7eed81fa2 100644 --- a/libs/apps/compat/compat_test.go +++ b/libs/depversions/compat_test.go @@ -1,4 +1,4 @@ -package compat +package depversions import ( "context" @@ -46,11 +46,11 @@ func testContext(t *testing.T) context.Context { func testManifest() Manifest { return Manifest{ - "next": {Appkit: "0.27.0", Skills: "0.1.5"}, - "0.296.0": {Appkit: "0.27.0", Skills: "0.1.5"}, - "0.295.0": {Appkit: "0.27.0", Skills: "0.1.5"}, - "0.290.0": {Appkit: "0.24.0", Skills: "0.1.5"}, - "0.288.0": {Appkit: "0.24.0", Skills: "0.1.4"}, + "next": {AppKit: "0.27.0", Skills: "0.1.5"}, + "0.296.0": {AppKit: "0.27.0", Skills: "0.1.5"}, + "0.295.0": {AppKit: "0.27.0", Skills: "0.1.5"}, + "0.290.0": {AppKit: "0.24.0", Skills: "0.1.5"}, + "0.288.0": {AppKit: "0.24.0", Skills: "0.1.4"}, } } @@ -58,7 +58,7 @@ func TestResolve_ExactMatch(t *testing.T) { m := testManifest() entry, err := Resolve(m, "0.296.0") require.NoError(t, err) - assert.Equal(t, "0.27.0", entry.Appkit) + assert.Equal(t, "0.27.0", entry.AppKit) assert.Equal(t, "0.1.5", entry.Skills) } @@ -67,20 +67,20 @@ func TestResolve_NearestLower(t *testing.T) { // 0.293.0 is between 0.290.0 and 0.295.0 → should use 0.290.0's entry entry, err := Resolve(m, "0.293.0") require.NoError(t, err) - assert.Equal(t, "0.24.0", entry.Appkit) + assert.Equal(t, "0.24.0", entry.AppKit) assert.Equal(t, "0.1.5", entry.Skills) } func TestResolve_NewerThanAll(t *testing.T) { m := Manifest{ - "next": {Appkit: "0.99.0", Skills: "0.9.9"}, - "0.296.0": {Appkit: "0.27.0", Skills: "0.1.5"}, - "0.290.0": {Appkit: "0.24.0", Skills: "0.1.5"}, + "next": {AppKit: "0.99.0", Skills: "0.9.9"}, + "0.296.0": {AppKit: "0.27.0", Skills: "0.1.5"}, + "0.290.0": {AppKit: "0.24.0", Skills: "0.1.5"}, } entry, err := Resolve(m, "0.300.0") require.NoError(t, err) // Nearest-lower returns the highest versioned entry, not "next". - assert.Equal(t, "0.27.0", entry.Appkit) + assert.Equal(t, "0.27.0", entry.AppKit) assert.Equal(t, "0.1.5", entry.Skills) } @@ -88,7 +88,7 @@ func TestResolve_DevBuild(t *testing.T) { m := testManifest() entry, err := Resolve(m, "0.0.0-dev+abc123def") require.NoError(t, err) - assert.Equal(t, "0.27.0", entry.Appkit) + assert.Equal(t, "0.27.0", entry.AppKit) assert.Equal(t, "0.1.5", entry.Skills) } @@ -96,18 +96,18 @@ func TestResolve_OlderThanAll(t *testing.T) { m := testManifest() entry, err := Resolve(m, "0.280.0") require.NoError(t, err) - // Falls back to "next" (best effort) - assert.Equal(t, "0.27.0", entry.Appkit) - assert.Equal(t, "0.1.5", entry.Skills) + // Uses the lowest (oldest) manifest entry as closest match. + assert.Equal(t, "0.24.0", entry.AppKit) + assert.Equal(t, "0.1.4", entry.Skills) } func TestResolve_OnlyNextKey(t *testing.T) { m := Manifest{ - "next": {Appkit: "0.27.0", Skills: "0.1.5"}, + "next": {AppKit: "0.27.0", Skills: "0.1.5"}, } entry, err := Resolve(m, "0.296.0") require.NoError(t, err) - assert.Equal(t, "0.27.0", entry.Appkit) + assert.Equal(t, "0.27.0", entry.AppKit) assert.Equal(t, "0.1.5", entry.Skills) } @@ -115,7 +115,7 @@ func TestResolve_LowestEntryExactMatch(t *testing.T) { m := testManifest() entry, err := Resolve(m, "0.288.0") require.NoError(t, err) - assert.Equal(t, "0.24.0", entry.Appkit) + assert.Equal(t, "0.24.0", entry.AppKit) assert.Equal(t, "0.1.4", entry.Skills) } @@ -128,7 +128,7 @@ func TestResolve_EmptyManifest(t *testing.T) { func TestResolve_MissingNextKey(t *testing.T) { m := Manifest{ - "0.296.0": {Appkit: "0.27.0", Skills: "0.1.5"}, + "0.296.0": {AppKit: "0.27.0", Skills: "0.1.5"}, } _, err := Resolve(m, "0.296.0") assert.Error(t, err) @@ -138,8 +138,8 @@ func TestResolve_MissingNextKey(t *testing.T) { func TestFetchManifest_RemoteSuccess(t *testing.T) { ctx := testContext(t) want := Manifest{ - "next": {Appkit: "0.99.0", Skills: "0.9.9"}, - "0.296.0": {Appkit: "0.99.0", Skills: "0.9.9"}, + "next": {AppKit: "0.99.0", Skills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", Skills: "0.9.9"}, } body, _ := json.Marshal(want) @@ -154,11 +154,9 @@ func TestFetchManifest_RemoteSuccess(t *testing.T) { result, err := FetchManifest(ctx) require.NoError(t, err) assert.True(t, called, "test server should have been called") - assert.Equal(t, "0.99.0", result["next"].Appkit) + assert.Equal(t, "0.99.0", result["next"].AppKit) } -// With {} as the embedded manifest, a remote failure means both remote and -// embedded fail → FetchManifest returns an error. func TestFetchManifest_RemoteFailReturnsError(t *testing.T) { ctx := testContext(t) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -169,7 +167,7 @@ func TestFetchManifest_RemoteFailReturnsError(t *testing.T) { _, err := FetchManifest(ctx) assert.Error(t, err) - assert.Contains(t, err.Error(), "empty compatibility manifest") + assert.Contains(t, err.Error(), "HTTP 500") } func TestFetchManifest_RemoteReturnsInvalidJSON(t *testing.T) { @@ -182,14 +180,14 @@ func TestFetchManifest_RemoteReturnsInvalidJSON(t *testing.T) { _, err := FetchManifest(ctx) assert.Error(t, err) - assert.Contains(t, err.Error(), "empty compatibility manifest") + assert.Contains(t, err.Error(), "invalid manifest JSON") } func TestFetchManifest_CacheHit(t *testing.T) { ctx := testContext(t) want := Manifest{ - "next": {Appkit: "0.99.0", Skills: "0.9.9"}, - "0.296.0": {Appkit: "0.99.0", Skills: "0.9.9"}, + "next": {AppKit: "0.99.0", Skills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", Skills: "0.9.9"}, } body, _ := json.Marshal(want) @@ -204,12 +202,12 @@ func TestFetchManifest_CacheHit(t *testing.T) { // First call: populates cache. result1, err := FetchManifest(ctx) require.NoError(t, err) - assert.Equal(t, "0.99.0", result1["next"].Appkit) + assert.Equal(t, "0.99.0", result1["next"].AppKit) // Second call: should come from cache, not hitting the server again. result2, err := FetchManifest(ctx) require.NoError(t, err) - assert.Equal(t, "0.99.0", result2["next"].Appkit) + assert.Equal(t, "0.99.0", result2["next"].AppKit) assert.Equal(t, int32(1), callCount.Load(), "server should only be called once; second call should be a cache hit") } @@ -217,8 +215,8 @@ func TestFetchManifest_CacheHit(t *testing.T) { func TestFetchManifest_RetryOnTransientError(t *testing.T) { ctx := testContext(t) want := Manifest{ - "next": {Appkit: "0.99.0", Skills: "0.9.9"}, - "0.296.0": {Appkit: "0.99.0", Skills: "0.9.9"}, + "next": {AppKit: "0.99.0", Skills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", Skills: "0.9.9"}, } body, _ := json.Marshal(want) @@ -236,7 +234,7 @@ func TestFetchManifest_RetryOnTransientError(t *testing.T) { result, err := FetchManifest(ctx) require.NoError(t, err) - assert.Equal(t, "0.99.0", result["next"].Appkit) + assert.Equal(t, "0.99.0", result["next"].AppKit) assert.Equal(t, int32(2), callCount.Load(), "should have retried after first failure") } @@ -244,8 +242,8 @@ func TestParseManifest_Valid(t *testing.T) { data := `{"next":{"appkit":"0.27.0","skills":"0.1.5"},"0.296.0":{"appkit":"0.27.0","skills":"0.1.5"}}` m, err := parseManifest([]byte(data)) require.NoError(t, err) - assert.Equal(t, "0.27.0", m["next"].Appkit) - assert.Equal(t, "0.27.0", m["0.296.0"].Appkit) + assert.Equal(t, "0.27.0", m["next"].AppKit) + assert.Equal(t, "0.27.0", m["0.296.0"].AppKit) } func TestParseManifest_InvalidJSON(t *testing.T) { @@ -266,3 +264,36 @@ func TestParseManifest_Empty(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "empty compatibility manifest") } + +func TestResolveEntry_RemoteSuccess(t *testing.T) { + ctx := testContext(t) + want := Manifest{ + "next": {AppKit: "0.99.0", Skills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", Skills: "0.9.9"}, + } + body, _ := json.Marshal(want) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(body) + })) + defer srv.Close() + redirectToServer(t, srv) + + entry, err := resolveEntry(ctx, "0.296.0") + require.NoError(t, err) + assert.Equal(t, "0.99.0", entry.AppKit) + assert.Equal(t, "0.9.9", entry.Skills) +} + +func TestResolveEntry_RemoteFailNoPinnedEntry(t *testing.T) { + ctx := testContext(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + redirectToServer(t, srv) + + _, err := resolveEntry(ctx, "0.296.0") + assert.Error(t, err) + assert.Contains(t, err.Error(), "manifest fetch failed and no build-time versions available") +} From c57909f6b0c2da2d4f6d9df41edcd07c4bf1bc8a Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Thu, 30 Apr 2026 17:19:42 +0200 Subject: [PATCH 06/10] refactor: rename Skills to AgentSkills, compat.go to depversions.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename Entry.Skills → Entry.AgentSkills (JSON tag stays "skills") - Rename DepVersions.Skills → DepVersions.AgentSkills - Rename compat.go → depversions.go to match package name - Verified pin tool handles missing manifest gracefully (404 → warning, placeholder unchanged) Co-authored-by: Isaac --- internal/build/dep_versions.go | 4 +- internal/pindepversions/main.go | 2 +- .../depversions/{compat.go => depversions.go} | 12 ++--- .../{compat_test.go => depversions_test.go} | 52 +++++++++---------- 4 files changed, 35 insertions(+), 35 deletions(-) rename libs/depversions/{compat.go => depversions.go} (95%) rename libs/depversions/{compat_test.go => depversions_test.go} (84%) diff --git a/internal/build/dep_versions.go b/internal/build/dep_versions.go index c4801c2361..7ad87c2b81 100644 --- a/internal/build/dep_versions.go +++ b/internal/build/dep_versions.go @@ -11,8 +11,8 @@ var depVersionsJSON []byte // DepVersions holds build-time resolved dependency versions. type DepVersions struct { - AppKit string `json:"appkit"` - Skills string `json:"skills"` + AppKit string `json:"appkit"` + AgentSkills string `json:"skills"` } var depVersions = sync.OnceValue(func() DepVersions { diff --git a/internal/pindepversions/main.go b/internal/pindepversions/main.go index 03e3602937..b168b95d4d 100644 --- a/internal/pindepversions/main.go +++ b/internal/pindepversions/main.go @@ -42,5 +42,5 @@ func main() { fmt.Fprintf(os.Stderr, "error: write %s: %v\n", *output, err) os.Exit(1) } - fmt.Fprintf(os.Stderr, "Pinned for CLI %s: appkit=%s skills=%s\n", *version, entry.AppKit, entry.Skills) + fmt.Fprintf(os.Stderr, "Pinned for CLI %s: appkit=%s skills=%s\n", *version, entry.AppKit, entry.AgentSkills) } diff --git a/libs/depversions/compat.go b/libs/depversions/depversions.go similarity index 95% rename from libs/depversions/compat.go rename to libs/depversions/depversions.go index d0df1ae496..8824b53f4c 100644 --- a/libs/depversions/compat.go +++ b/libs/depversions/depversions.go @@ -37,10 +37,10 @@ type manifestFingerprint struct { URL string `json:"url"` } -// Entry maps a CLI version to compatible AppKit and skills versions. +// Entry maps a CLI version to compatible AppKit and Agent Skills versions. type Entry struct { - AppKit string `json:"appkit"` - Skills string `json:"skills"` + AppKit string `json:"appkit"` + AgentSkills string `json:"skills"` } // Manifest is the compatibility manifest: a map of CLI version strings to entries. @@ -131,8 +131,8 @@ func resolveEntry(ctx context.Context, cliVersion string) (Entry, error) { dv := build.GetDepVersions() if dv.AppKit != "" { - log.Debugf(ctx, "Using build-time dep versions: appkit=%s skills=%s", dv.AppKit, dv.Skills) - return Entry{AppKit: dv.AppKit, Skills: dv.Skills}, nil + log.Debugf(ctx, "Using build-time dep versions: appkit=%s skills=%s", dv.AppKit, dv.AgentSkills) + return Entry{AppKit: dv.AppKit, AgentSkills: dv.AgentSkills}, nil } if fetchErr != nil { @@ -156,7 +156,7 @@ func ResolveAgentSkillsVersion(ctx context.Context) (string, error) { if err != nil { return "", err } - return entry.Skills, nil + return entry.AgentSkills, nil } // fetchRemoteWithRetry wraps fetchRemote with retries. diff --git a/libs/depversions/compat_test.go b/libs/depversions/depversions_test.go similarity index 84% rename from libs/depversions/compat_test.go rename to libs/depversions/depversions_test.go index e7eed81fa2..8559a55599 100644 --- a/libs/depversions/compat_test.go +++ b/libs/depversions/depversions_test.go @@ -46,11 +46,11 @@ func testContext(t *testing.T) context.Context { func testManifest() Manifest { return Manifest{ - "next": {AppKit: "0.27.0", Skills: "0.1.5"}, - "0.296.0": {AppKit: "0.27.0", Skills: "0.1.5"}, - "0.295.0": {AppKit: "0.27.0", Skills: "0.1.5"}, - "0.290.0": {AppKit: "0.24.0", Skills: "0.1.5"}, - "0.288.0": {AppKit: "0.24.0", Skills: "0.1.4"}, + "next": {AppKit: "0.27.0", AgentSkills: "0.1.5"}, + "0.296.0": {AppKit: "0.27.0", AgentSkills: "0.1.5"}, + "0.295.0": {AppKit: "0.27.0", AgentSkills: "0.1.5"}, + "0.290.0": {AppKit: "0.24.0", AgentSkills: "0.1.5"}, + "0.288.0": {AppKit: "0.24.0", AgentSkills: "0.1.4"}, } } @@ -59,7 +59,7 @@ func TestResolve_ExactMatch(t *testing.T) { entry, err := Resolve(m, "0.296.0") require.NoError(t, err) assert.Equal(t, "0.27.0", entry.AppKit) - assert.Equal(t, "0.1.5", entry.Skills) + assert.Equal(t, "0.1.5", entry.AgentSkills) } func TestResolve_NearestLower(t *testing.T) { @@ -68,20 +68,20 @@ func TestResolve_NearestLower(t *testing.T) { entry, err := Resolve(m, "0.293.0") require.NoError(t, err) assert.Equal(t, "0.24.0", entry.AppKit) - assert.Equal(t, "0.1.5", entry.Skills) + assert.Equal(t, "0.1.5", entry.AgentSkills) } func TestResolve_NewerThanAll(t *testing.T) { m := Manifest{ - "next": {AppKit: "0.99.0", Skills: "0.9.9"}, - "0.296.0": {AppKit: "0.27.0", Skills: "0.1.5"}, - "0.290.0": {AppKit: "0.24.0", Skills: "0.1.5"}, + "next": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, + "0.296.0": {AppKit: "0.27.0", AgentSkills: "0.1.5"}, + "0.290.0": {AppKit: "0.24.0", AgentSkills: "0.1.5"}, } entry, err := Resolve(m, "0.300.0") require.NoError(t, err) // Nearest-lower returns the highest versioned entry, not "next". assert.Equal(t, "0.27.0", entry.AppKit) - assert.Equal(t, "0.1.5", entry.Skills) + assert.Equal(t, "0.1.5", entry.AgentSkills) } func TestResolve_DevBuild(t *testing.T) { @@ -89,7 +89,7 @@ func TestResolve_DevBuild(t *testing.T) { entry, err := Resolve(m, "0.0.0-dev+abc123def") require.NoError(t, err) assert.Equal(t, "0.27.0", entry.AppKit) - assert.Equal(t, "0.1.5", entry.Skills) + assert.Equal(t, "0.1.5", entry.AgentSkills) } func TestResolve_OlderThanAll(t *testing.T) { @@ -98,17 +98,17 @@ func TestResolve_OlderThanAll(t *testing.T) { require.NoError(t, err) // Uses the lowest (oldest) manifest entry as closest match. assert.Equal(t, "0.24.0", entry.AppKit) - assert.Equal(t, "0.1.4", entry.Skills) + assert.Equal(t, "0.1.4", entry.AgentSkills) } func TestResolve_OnlyNextKey(t *testing.T) { m := Manifest{ - "next": {AppKit: "0.27.0", Skills: "0.1.5"}, + "next": {AppKit: "0.27.0", AgentSkills: "0.1.5"}, } entry, err := Resolve(m, "0.296.0") require.NoError(t, err) assert.Equal(t, "0.27.0", entry.AppKit) - assert.Equal(t, "0.1.5", entry.Skills) + assert.Equal(t, "0.1.5", entry.AgentSkills) } func TestResolve_LowestEntryExactMatch(t *testing.T) { @@ -116,7 +116,7 @@ func TestResolve_LowestEntryExactMatch(t *testing.T) { entry, err := Resolve(m, "0.288.0") require.NoError(t, err) assert.Equal(t, "0.24.0", entry.AppKit) - assert.Equal(t, "0.1.4", entry.Skills) + assert.Equal(t, "0.1.4", entry.AgentSkills) } func TestResolve_EmptyManifest(t *testing.T) { @@ -128,7 +128,7 @@ func TestResolve_EmptyManifest(t *testing.T) { func TestResolve_MissingNextKey(t *testing.T) { m := Manifest{ - "0.296.0": {AppKit: "0.27.0", Skills: "0.1.5"}, + "0.296.0": {AppKit: "0.27.0", AgentSkills: "0.1.5"}, } _, err := Resolve(m, "0.296.0") assert.Error(t, err) @@ -138,8 +138,8 @@ func TestResolve_MissingNextKey(t *testing.T) { func TestFetchManifest_RemoteSuccess(t *testing.T) { ctx := testContext(t) want := Manifest{ - "next": {AppKit: "0.99.0", Skills: "0.9.9"}, - "0.296.0": {AppKit: "0.99.0", Skills: "0.9.9"}, + "next": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, } body, _ := json.Marshal(want) @@ -186,8 +186,8 @@ func TestFetchManifest_RemoteReturnsInvalidJSON(t *testing.T) { func TestFetchManifest_CacheHit(t *testing.T) { ctx := testContext(t) want := Manifest{ - "next": {AppKit: "0.99.0", Skills: "0.9.9"}, - "0.296.0": {AppKit: "0.99.0", Skills: "0.9.9"}, + "next": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, } body, _ := json.Marshal(want) @@ -215,8 +215,8 @@ func TestFetchManifest_CacheHit(t *testing.T) { func TestFetchManifest_RetryOnTransientError(t *testing.T) { ctx := testContext(t) want := Manifest{ - "next": {AppKit: "0.99.0", Skills: "0.9.9"}, - "0.296.0": {AppKit: "0.99.0", Skills: "0.9.9"}, + "next": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, } body, _ := json.Marshal(want) @@ -268,8 +268,8 @@ func TestParseManifest_Empty(t *testing.T) { func TestResolveEntry_RemoteSuccess(t *testing.T) { ctx := testContext(t) want := Manifest{ - "next": {AppKit: "0.99.0", Skills: "0.9.9"}, - "0.296.0": {AppKit: "0.99.0", Skills: "0.9.9"}, + "next": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, } body, _ := json.Marshal(want) @@ -282,7 +282,7 @@ func TestResolveEntry_RemoteSuccess(t *testing.T) { entry, err := resolveEntry(ctx, "0.296.0") require.NoError(t, err) assert.Equal(t, "0.99.0", entry.AppKit) - assert.Equal(t, "0.9.9", entry.Skills) + assert.Equal(t, "0.9.9", entry.AgentSkills) } func TestResolveEntry_RemoteFailNoPinnedEntry(t *testing.T) { From c6f8047dba344f447362ce453181e10e4d99d3a7 Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Thu, 30 Apr 2026 18:11:47 +0200 Subject: [PATCH 07/10] feat: show default AppKit version in --version flag help Use build-time pinned version from dep_versions.json in the --version flag help text. Release builds show the actual default; dev builds show a generic message since no version is pinned. Co-authored-by: Isaac --- cmd/apps/init.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/apps/init.go b/cmd/apps/init.go index 754c3fe985..467d530a5d 100644 --- a/cmd/apps/init.go +++ b/cmd/apps/init.go @@ -19,6 +19,7 @@ import ( "github.com/databricks/cli/cmd/root" "github.com/databricks/cli/experimental/aitools/lib/agents" "github.com/databricks/cli/experimental/aitools/lib/installer" + "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/depversions" "github.com/databricks/cli/libs/apps/generator" "github.com/databricks/cli/libs/apps/initializer" @@ -169,7 +170,11 @@ Environment variables: cmd.Flags().StringVar(&templatePath, "template", "", "Template path (local directory or GitHub URL)") cmd.Flags().StringVar(&branch, "branch", "", "Git branch or tag (for GitHub templates, mutually exclusive with --version)") - cmd.Flags().StringVar(&version, "version", "", "AppKit version to use (resolved from compatibility manifest, use 'latest' for main branch)") + versionDesc := "AppKit version to use (use 'latest' for main branch)" + if v := build.GetDepVersions().AppKit; v != "" { + versionDesc = fmt.Sprintf("AppKit version to use (default: %s, use 'latest' for main branch)", v) + } + cmd.Flags().StringVar(&version, "version", "", versionDesc) cmd.Flags().StringVar(&name, "name", "", "Project name (prompts if not provided)") cmd.Flags().StringVar(&warehouseID, "warehouse-id", "", "SQL warehouse ID") _ = cmd.Flags().MarkDeprecated("warehouse-id", "use --set .sql-warehouse.id= instead") From 146071adb41076254a8d825db8842cd1b3f5719e Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Thu, 30 Apr 2026 18:16:05 +0200 Subject: [PATCH 08/10] fix: address lint issues from golangci-lint - Replace sort.Slice with slices.SortFunc (forbidigo) - Check w.Write error returns in test handlers (errcheck) Co-authored-by: Isaac --- cmd/apps/init.go | 8 ++++---- libs/depversions/depversions.go | 13 +++++++------ libs/depversions/depversions_test.go | 10 +++++----- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/cmd/apps/init.go b/cmd/apps/init.go index 467d530a5d..5f6b3ed25c 100644 --- a/cmd/apps/init.go +++ b/cmd/apps/init.go @@ -20,13 +20,13 @@ import ( "github.com/databricks/cli/experimental/aitools/lib/agents" "github.com/databricks/cli/experimental/aitools/lib/installer" "github.com/databricks/cli/internal/build" - "github.com/databricks/cli/libs/depversions" "github.com/databricks/cli/libs/apps/generator" "github.com/databricks/cli/libs/apps/initializer" "github.com/databricks/cli/libs/apps/manifest" "github.com/databricks/cli/libs/apps/prompt" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/depversions" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/git" "github.com/databricks/cli/libs/log" @@ -40,7 +40,7 @@ const ( appkitTemplateDir = "template" appkitDefaultBranch = "main" appkitTemplateTagPfx = "template-v" - defaultProfile = "DEFAULT" + defaultProfile = "DEFAULT" ) // normalizeVersion converts a version string to the template tag format "template-vX.X.X". @@ -787,7 +787,7 @@ func runCreate(ctx context.Context, opts createOptions) error { return fmt.Errorf("could not resolve AppKit template version: %w. Use --version to specify a version manually", err) } gitRef = normalizeVersion(appkitVersion) - cmdio.LogString(ctx, fmt.Sprintf("Using AppKit template version %s", appkitVersion)) + cmdio.LogString(ctx, "Using AppKit template version "+appkitVersion) skillsVersion, err := depversions.ResolveAgentSkillsVersion(ctx) if err == nil { @@ -1152,7 +1152,7 @@ func runCreate(ctx context.Context, opts createOptions) error { skillsCtx := ctx if resolvedSkillsVersion != "" { skillsCtx = env.Set(ctx, "DATABRICKS_SKILLS_REF", "v"+resolvedSkillsVersion) - cmdio.LogString(ctx, fmt.Sprintf("Using skills version %s", resolvedSkillsVersion)) + cmdio.LogString(ctx, "Using skills version "+resolvedSkillsVersion) } // Recommend skills installation if coding agents are detected without skills. diff --git a/libs/depversions/depversions.go b/libs/depversions/depversions.go index 8824b53f4c..dca8b6c61f 100644 --- a/libs/depversions/depversions.go +++ b/libs/depversions/depversions.go @@ -3,10 +3,11 @@ package depversions import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" - "sort" + "slices" "strings" "time" @@ -70,7 +71,7 @@ func FetchManifest(ctx context.Context) (Manifest, error) { // 4. If CLI is older than all entries, use the lowest (oldest) entry. func Resolve(m Manifest, cliVersion string) (Entry, error) { if len(m) == 0 { - return Entry{}, fmt.Errorf("empty compatibility manifest") + return Entry{}, errors.New("empty compatibility manifest") } next, ok := m[nextKey] @@ -97,8 +98,8 @@ func Resolve(m Manifest, cliVersion string) (Entry, error) { } // Sort descending by semver. The semver package requires a "v" prefix. - sort.Slice(versions, func(i, j int) bool { - return semver.Compare("v"+versions[i], "v"+versions[j]) > 0 + slices.SortFunc(versions, func(a, b string) int { + return semver.Compare("v"+b, "v"+a) }) // Find the nearest lower version. @@ -138,7 +139,7 @@ func resolveEntry(ctx context.Context, cliVersion string) (Entry, error) { if fetchErr != nil { return Entry{}, fmt.Errorf("manifest fetch failed and no build-time versions available: %w", fetchErr) } - return Entry{}, fmt.Errorf("no compatible versions available") + return Entry{}, errors.New("no compatible versions available") } // ResolveAppKitVersion resolves the AppKit template version for the current CLI. @@ -212,7 +213,7 @@ func parseManifest(data []byte) (Manifest, error) { return nil, fmt.Errorf("invalid manifest JSON: %w", err) } if len(m) == 0 { - return nil, fmt.Errorf("empty compatibility manifest") + return nil, errors.New("empty compatibility manifest") } if _, ok := m[nextKey]; !ok { return nil, fmt.Errorf("compatibility manifest missing %q key", nextKey) diff --git a/libs/depversions/depversions_test.go b/libs/depversions/depversions_test.go index 8559a55599..e30228d7a6 100644 --- a/libs/depversions/depversions_test.go +++ b/libs/depversions/depversions_test.go @@ -146,7 +146,7 @@ func TestFetchManifest_RemoteSuccess(t *testing.T) { var called bool srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { called = true - w.Write(body) + _, _ = w.Write(body) })) defer srv.Close() redirectToServer(t, srv) @@ -173,7 +173,7 @@ func TestFetchManifest_RemoteFailReturnsError(t *testing.T) { func TestFetchManifest_RemoteReturnsInvalidJSON(t *testing.T) { ctx := testContext(t) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte("not json at all")) + _, _ = w.Write([]byte("not json at all")) })) defer srv.Close() redirectToServer(t, srv) @@ -194,7 +194,7 @@ func TestFetchManifest_CacheHit(t *testing.T) { var callCount atomic.Int32 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { callCount.Add(1) - w.Write(body) + _, _ = w.Write(body) })) defer srv.Close() redirectToServer(t, srv) @@ -227,7 +227,7 @@ func TestFetchManifest_RetryOnTransientError(t *testing.T) { w.WriteHeader(http.StatusInternalServerError) return } - w.Write(body) + _, _ = w.Write(body) })) defer srv.Close() redirectToServer(t, srv) @@ -274,7 +274,7 @@ func TestResolveEntry_RemoteSuccess(t *testing.T) { body, _ := json.Marshal(want) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write(body) + _, _ = w.Write(body) })) defer srv.Close() redirectToServer(t, srv) From 336dc3e0879017a2b88c8de50861b801fd8b7904 Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Thu, 30 Apr 2026 18:25:16 +0200 Subject: [PATCH 09/10] refactor: resolve skills version from manifest, remove SKILLS_VERSION fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - GetSkillsRef now resolves from compatibility manifest instead of falling back to hardcoded SKILLS_VERSION file - Delete SKILLS_VERSION and version.go — manifest is sole source of truth - Remove DATABRICKS_SKILLS_REF env var override from apps init (GetSkillsRef handles manifest resolution internally) - Update all callers to handle GetSkillsRef error return - Tests use t.Setenv("DATABRICKS_SKILLS_REF", ...) to bypass manifest Co-authored-by: Isaac --- cmd/apps/init.go | 19 ++------------ experimental/aitools/cmd/list.go | 5 +++- experimental/aitools/cmd/version.go | 6 ++++- .../aitools/lib/installer/SKILLS_VERSION | 1 - .../aitools/lib/installer/installer.go | 20 +++++++++----- .../aitools/lib/installer/installer_test.go | 26 ++++++++++++++++--- .../aitools/lib/installer/uninstall_test.go | 2 ++ experimental/aitools/lib/installer/update.go | 5 +++- .../aitools/lib/installer/update_test.go | 12 ++++++++- experimental/aitools/lib/installer/version.go | 14 ---------- 10 files changed, 65 insertions(+), 45 deletions(-) delete mode 100644 experimental/aitools/lib/installer/SKILLS_VERSION delete mode 100644 experimental/aitools/lib/installer/version.go diff --git a/cmd/apps/init.go b/cmd/apps/init.go index 5f6b3ed25c..007999d237 100644 --- a/cmd/apps/init.go +++ b/cmd/apps/init.go @@ -773,7 +773,6 @@ func runCreate(ctx context.Context, opts createOptions) error { // Resolve the git reference (branch/tag) to use for default appkit template gitRef := opts.branch usingDefaultTemplate := templateSrc == "" - var resolvedSkillsVersion string if usingDefaultTemplate { // Using default appkit template - resolve version switch { @@ -788,11 +787,6 @@ func runCreate(ctx context.Context, opts createOptions) error { } gitRef = normalizeVersion(appkitVersion) cmdio.LogString(ctx, "Using AppKit template version "+appkitVersion) - - skillsVersion, err := depversions.ResolveAgentSkillsVersion(ctx) - if err == nil { - resolvedSkillsVersion = skillsVersion - } } templateSrc = appkitRepoURL } @@ -1146,22 +1140,13 @@ func runCreate(ctx context.Context, opts createOptions) error { prompt.PrintSetupNotes(ctx, notes) } - // If the manifest resolved a skills version, set it on the context so the - // skills installer uses the manifest-compatible version instead of its - // embedded SKILLS_VERSION default. - skillsCtx := ctx - if resolvedSkillsVersion != "" { - skillsCtx = env.Set(ctx, "DATABRICKS_SKILLS_REF", "v"+resolvedSkillsVersion) - cmdio.LogString(ctx, "Using skills version "+resolvedSkillsVersion) - } - // Recommend skills installation if coding agents are detected without skills. // In flags mode, only print a hint — never prompt interactively. if flagsMode { - if !agents.HasDatabricksSkillsInstalled(skillsCtx) { + if !agents.HasDatabricksSkillsInstalled(ctx) { cmdio.LogString(ctx, "Tip: coding agents detected without Databricks skills. Run 'databricks experimental aitools skills install' to install them.") } - } else if err := agents.RecommendSkillsInstall(skillsCtx, installer.InstallAllSkills); err != nil { + } else if err := agents.RecommendSkillsInstall(ctx, installer.InstallAllSkills); err != nil { log.Warnf(ctx, "Skills recommendation failed: %v", err) } diff --git a/experimental/aitools/cmd/list.go b/experimental/aitools/cmd/list.go index 1be1538c9a..8f665d7ad1 100644 --- a/experimental/aitools/cmd/list.go +++ b/experimental/aitools/cmd/list.go @@ -48,7 +48,10 @@ func newListCmd() *cobra.Command { func defaultListSkills(cmd *cobra.Command, scope string) error { ctx := cmd.Context() - ref := installer.GetSkillsRef(ctx) + ref, err := installer.GetSkillsRef(ctx) + if err != nil { + return err + } src := &installer.GitHubManifestSource{} manifest, err := src.FetchManifest(ctx, ref) diff --git a/experimental/aitools/cmd/version.go b/experimental/aitools/cmd/version.go index 67c38fec42..17faa468a6 100644 --- a/experimental/aitools/cmd/version.go +++ b/experimental/aitools/cmd/version.go @@ -7,6 +7,7 @@ import ( "github.com/databricks/cli/experimental/aitools/lib/installer" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/log" "github.com/spf13/cobra" ) @@ -44,7 +45,10 @@ func newVersionCmd() *cobra.Command { return nil } - latestRef := installer.GetSkillsRef(ctx) + latestRef, err := installer.GetSkillsRef(ctx) + if err != nil { + log.Debugf(ctx, "could not resolve skills version: %v", err) + } bothScopes := globalState != nil && projectState != nil cmdio.LogString(ctx, "Databricks AI Tools:") diff --git a/experimental/aitools/lib/installer/SKILLS_VERSION b/experimental/aitools/lib/installer/SKILLS_VERSION deleted file mode 100644 index 027a383a35..0000000000 --- a/experimental/aitools/lib/installer/SKILLS_VERSION +++ /dev/null @@ -1 +0,0 @@ -v0.1.5 diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go index f9d8d64741..0c38d470ce 100644 --- a/experimental/aitools/lib/installer/installer.go +++ b/experimental/aitools/lib/installer/installer.go @@ -17,6 +17,7 @@ import ( "github.com/databricks/cli/experimental/aitools/lib/agents" "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/depversions" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/log" "github.com/fatih/color" @@ -33,13 +34,17 @@ const ( // It is a package-level var so tests can replace it with a mock. var fetchFileFn = fetchSkillFile -// GetSkillsRef returns the skills repo ref to use. If DATABRICKS_SKILLS_REF -// is set, it returns that value; otherwise it returns the default ref. -func GetSkillsRef(ctx context.Context) string { +// GetSkillsRef returns the skills repo ref to use. +// Resolution order: DATABRICKS_SKILLS_REF env var → compatibility manifest → error. +func GetSkillsRef(ctx context.Context) (string, error) { if ref := env.Get(ctx, "DATABRICKS_SKILLS_REF"); ref != "" { - return ref + return ref, nil } - return defaultSkillsRepoRef + v, err := depversions.ResolveAgentSkillsVersion(ctx) + if err != nil { + return "", fmt.Errorf("could not resolve skills version: %w", err) + } + return "v" + v, nil } // Manifest describes the skills manifest fetched from the skills repo. @@ -93,7 +98,10 @@ func fetchSkillFile(ctx context.Context, ref, skillName, filePath string) ([]byt // 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 := GetSkillsRef(ctx) + ref, err := GetSkillsRef(ctx) + if err != nil { + return err + } manifest, err := src.FetchManifest(ctx, ref) if err != nil { return err diff --git a/experimental/aitools/lib/installer/installer_test.go b/experimental/aitools/lib/installer/installer_test.go index 162c7abb8a..df3bf8f6b0 100644 --- a/experimental/aitools/lib/installer/installer_test.go +++ b/experimental/aitools/lib/installer/installer_test.go @@ -17,6 +17,8 @@ import ( "github.com/stretchr/testify/require" ) +const testSkillsRef = "v0.1.5" + // mockManifestSource is a test double for ManifestSource. type mockManifestSource struct { manifest *Manifest @@ -190,6 +192,7 @@ func TestInstallSkillsForAgentsWritesState(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) @@ -202,7 +205,7 @@ func TestInstallSkillsForAgentsWritesState(t *testing.T) { require.NoError(t, err) require.NotNil(t, state) assert.Equal(t, 1, state.SchemaVersion) - assert.Equal(t, defaultSkillsRepoRef, state.Release) + assert.Equal(t, testSkillsRef, state.Release) assert.Len(t, state.Skills, 2) assert.Equal(t, "0.1.0", state.Skills["databricks-sql"]) assert.Equal(t, "0.1.0", state.Skills["databricks-jobs"]) @@ -214,6 +217,7 @@ func TestInstallSkillForSingleWritesState(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) @@ -237,6 +241,7 @@ func TestInstallSkillsSpecificNotFound(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) @@ -252,6 +257,7 @@ func TestExperimentalSkillsSkippedByDefault(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) manifest := testManifest() manifest.Skills["databricks-experimental"] = SkillMeta{ @@ -280,6 +286,7 @@ func TestExperimentalSkillsIncludedWithFlag(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) manifest := testManifest() manifest.Skills["databricks-experimental"] = SkillMeta{ @@ -310,6 +317,7 @@ func TestMinCLIVersionSkipWithWarningForInstallAll(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) setBuildVersion(t, "0.200.0") // Capture log output to verify the warning. @@ -345,6 +353,7 @@ func TestMinCLIVersionHardErrorForInstallSingle(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) setBuildVersion(t, "0.200.0") manifest := testManifest() @@ -369,6 +378,7 @@ func TestIdempotentSecondInstallSkips(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) @@ -398,6 +408,7 @@ func TestIdempotentInstallUpdatesNewVersions(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) @@ -435,7 +446,7 @@ func TestIdempotentInstallUpdatesNewVersions(t *testing.T) { globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") state, err := LoadState(globalDir) require.NoError(t, err) - assert.Equal(t, defaultSkillsRepoRef, state.Release) + assert.Equal(t, testSkillsRef, state.Release) assert.Equal(t, "0.2.0", state.Skills["databricks-sql"]) } @@ -443,6 +454,7 @@ func TestLegacyDetectMessagePrinted(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Create skills on disk at canonical location but no state file. globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") @@ -461,6 +473,7 @@ func TestLegacyDetectLegacyDir(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Create skills in the legacy location. legacyDir := filepath.Join(tmp, ".databricks", "agent-skills") @@ -479,6 +492,7 @@ func TestIdempotentInstallReinstallsForNewAgent(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) src := &mockManifestSource{manifest: testManifest()} agent1 := testAgent(tmp) @@ -524,6 +538,7 @@ func TestLegacyTargetedInstallBlocked(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Create skills on disk at canonical location but no state file (legacy). globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") @@ -544,6 +559,7 @@ func TestLegacyFullInstallAllowed(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Create skills on disk at canonical location but no state file (legacy). globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") @@ -588,6 +604,7 @@ func TestInstallProjectScopeWritesState(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Use project dir as cwd. projectDir := filepath.Join(tmp, "myproject") @@ -605,7 +622,7 @@ func TestInstallProjectScopeWritesState(t *testing.T) { require.NoError(t, err) require.NotNil(t, state) assert.Equal(t, ScopeProject, state.Scope) - assert.Equal(t, defaultSkillsRepoRef, state.Release) + assert.Equal(t, testSkillsRef, state.Release) assert.Len(t, state.Skills, 2) assert.Contains(t, stderr.String(), "Installed 2 skills.") @@ -615,6 +632,7 @@ func TestInstallProjectScopeCreatesSymlinks(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) projectDir := filepath.Join(tmp, "myproject") require.NoError(t, os.MkdirAll(projectDir, 0o755)) @@ -657,6 +675,7 @@ func TestInstallProjectScopeFiltersIncompatibleAgents(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) projectDir := filepath.Join(tmp, "myproject") require.NoError(t, os.MkdirAll(projectDir, 0o755)) @@ -684,6 +703,7 @@ func TestInstallProjectScopeZeroCompatibleAgentsReturnsError(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) projectDir := filepath.Join(tmp, "myproject") require.NoError(t, os.MkdirAll(projectDir, 0o755)) diff --git a/experimental/aitools/lib/installer/uninstall_test.go b/experimental/aitools/lib/installer/uninstall_test.go index 6c7589f6f2..444fb773af 100644 --- a/experimental/aitools/lib/installer/uninstall_test.go +++ b/experimental/aitools/lib/installer/uninstall_test.go @@ -18,6 +18,7 @@ func installTestSkills(t *testing.T, tmp string) string { t.Helper() ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) @@ -48,6 +49,7 @@ func TestUninstallRemovesSymlinks(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Use two registry-based agents so uninstall can find them. // Create config dirs for claude-code and cursor (both in agents.Registry). diff --git a/experimental/aitools/lib/installer/update.go b/experimental/aitools/lib/installer/update.go index 663ad5e908..0aa260f115 100644 --- a/experimental/aitools/lib/installer/update.go +++ b/experimental/aitools/lib/installer/update.go @@ -81,7 +81,10 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent return nil, errors.New("no skills installed. Run 'databricks experimental aitools install' to install") } - latestTag := GetSkillsRef(ctx) + latestTag, err := GetSkillsRef(ctx) + if err != nil { + return nil, err + } if state.Release == latestTag && !opts.Force { cmdio.LogString(ctx, "Already up to date.") diff --git a/experimental/aitools/lib/installer/update_test.go b/experimental/aitools/lib/installer/update_test.go index 97e3014be6..3272a77bab 100644 --- a/experimental/aitools/lib/installer/update_test.go +++ b/experimental/aitools/lib/installer/update_test.go @@ -46,6 +46,7 @@ func TestUpdateAlreadyUpToDate(t *testing.T) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Install first. src := &mockManifestSource{manifest: testManifest()} @@ -68,6 +69,7 @@ func TestUpdateVersionDiffDetected(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Install with default ref. src := &mockManifestSource{manifest: testManifest()} @@ -108,6 +110,7 @@ func TestUpdateCheckDryRun(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Install with default ref. src := &mockManifestSource{manifest: testManifest()} @@ -148,13 +151,14 @@ func TestUpdateCheckDryRun(t *testing.T) { globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") state, err := LoadState(globalDir) require.NoError(t, err) - assert.Equal(t, defaultSkillsRepoRef, state.Release) + assert.Equal(t, testSkillsRef, state.Release) } func TestUpdateForceRedownloads(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Install v0.1.0. src := &mockManifestSource{manifest: testManifest()} @@ -182,6 +186,7 @@ func TestUpdateAutoAddsNewSkills(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Install with default ref. src := &mockManifestSource{manifest: testManifest()} @@ -218,6 +223,7 @@ func TestUpdateNoNewIgnoresNewSkills(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Install with default ref. src := &mockManifestSource{manifest: testManifest()} @@ -254,6 +260,7 @@ func TestUpdateOutputSortedAlphabetically(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Install with skills. src := &mockManifestSource{manifest: testManifest()} @@ -281,6 +288,7 @@ func TestUpdateSkillRemovedFromManifestWarning(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Capture log output to verify warning. var logBuf bytes.Buffer @@ -325,6 +333,7 @@ func TestUpdateSkipsExperimentalSkills(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) // Install with default ref (not experimental). src := &mockManifestSource{manifest: testManifest()} @@ -355,6 +364,7 @@ func TestUpdateSkipsMinCLIVersionSkills(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) setBuildVersion(t, "0.200.0") var logBuf bytes.Buffer diff --git a/experimental/aitools/lib/installer/version.go b/experimental/aitools/lib/installer/version.go deleted file mode 100644 index 0e942ca0a9..0000000000 --- a/experimental/aitools/lib/installer/version.go +++ /dev/null @@ -1,14 +0,0 @@ -package installer - -import ( - _ "embed" - "strings" -) - -//go:embed SKILLS_VERSION -var skillsVersionFile string - -// defaultSkillsRepoRef is the pinned tag of databricks/databricks-agent-skills. -// It is sourced from the SKILLS_VERSION file so automation can bump the pin -// with a single-line file edit instead of patching Go source. -var defaultSkillsRepoRef = strings.TrimSpace(skillsVersionFile) From 62b86450624f855fa0a16dd1b371a1608cca280f Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Thu, 30 Apr 2026 18:35:03 +0200 Subject: [PATCH 10/10] feat: print resolved skills version during aitools install Show "Using skills version X.Y.Z" before installation so the user knows which version was resolved from the compatibility manifest. Co-authored-by: Isaac --- experimental/aitools/lib/installer/installer.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go index 0c38d470ce..94c8bc86ab 100644 --- a/experimental/aitools/lib/installer/installer.go +++ b/experimental/aitools/lib/installer/installer.go @@ -102,6 +102,8 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent if err != nil { return err } + tag := strings.TrimPrefix(ref, "v") + cmdio.LogString(ctx, "Using skills version "+tag) manifest, err := src.FetchManifest(ctx, ref) if err != nil { return err