diff --git a/.agent/skills/bump-cli-compat/SKILL.md b/.agent/skills/bump-cli-compat/SKILL.md new file mode 100644 index 00000000000..a1fe768776e --- /dev/null +++ b/.agent/skills/bump-cli-compat/SKILL.md @@ -0,0 +1,132 @@ +--- +name: bump-cli-compat +description: "Bump cli-compat.json with new AppKit and Agent Skills versions, then create a PR. Use when the user says 'bump cli-compat', 'update cli-compat', 'bump compatibility manifest', 'new appkit release cli-compat', or wants to update the CLI compatibility manifest after an AppKit or Agent Skills release." +user-invocable: true +allowed-tools: Read, Edit, Write, Bash, Glob, Grep, AskUserQuestion +--- + +# Bump CLI Compatibility Manifest + +Updates `internal/build/cli-compat.json` with new AppKit and Agent Skills versions, validates the result, and creates a PR. + +## Arguments + +Parse the user's input for optional named flags: + +- `--appkit ` → AppKit version (e.g. `0.28.0`) +- `--skills ` → Agent Skills version (e.g. `0.1.6`) +- No args → auto-detect latest versions from GitHub tags + +Versions should be provided **without** the `v` prefix (e.g. `0.28.0`, not `v0.28.0`). If provided with the prefix, strip it. + +## Workflow + +### Step 1: Resolve versions + +If both `--appkit` and `--skills` versions were provided, skip to Step 2. + +For any missing version, fetch the latest tag from GitHub: + +```bash +# Latest appkit version (strip leading 'v') +gh api repos/databricks/appkit/tags --jq '.[0].name' | sed 's/^v//' + +# Latest skills version (strip leading 'v') +gh api repos/databricks/databricks-agent-skills/tags --jq '.[0].name' | sed 's/^v//' +``` + +Show the resolved versions to the user and ask: + +> The latest versions are: +> - AppKit: `{appkit_version}` +> - Agent Skills: `{skills_version}` +> +> Have these versions been evaluated (evals passed with no regressions)? + +**Do NOT proceed until the user confirms.** If the user says no or wants different versions, ask them to provide the correct versions. + +### Step 2: Validate tags exist + +Verify that the corresponding Git tags exist on GitHub. For AppKit, also validate the `template-v` tag (used by `apps init`): + +```bash +# AppKit release tag +gh api repos/databricks/appkit/git/ref/tags/v{appkit_version} --jq '.ref' + +# AppKit template tag (used by apps init) +gh api repos/databricks/appkit/git/ref/tags/template-v{appkit_version} --jq '.ref' + +# Agent Skills tag +gh api repos/databricks/databricks-agent-skills/git/ref/tags/v{skills_version} --jq '.ref' +``` + +If any tag doesn't exist, report the error and stop. + +### Step 3: Read current manifest + +Read `internal/build/cli-compat.json`. Note the current versions and the list of versioned entries. + +### Step 4: Update the manifest + +Update **all entries** (both `next` and all versioned CLI entries) to the new appkit and skills versions. This is the "no template changes" scenario — a simple search & replace. + +Write the updated `internal/build/cli-compat.json`. + +### Step 5: Validate + +Run the Go tests to ensure the manifest is well-formed: + +```bash +go test ./libs/clicompat/... -run TestEmbeddedManifest -v +``` + +If validation fails, show the errors and fix them before proceeding. + +### Step 6: Create branch, commit, and PR + +```bash +# Create a new branch from the current branch (or main) +git checkout -b bump-cli-compat-appkit-{appkit_version}-skills-{skills_version} + +# Stage and commit +git add internal/build/cli-compat.json +git commit -s -m "Bump cli-compat to appkit {appkit_version}, skills {skills_version}" + +# Push and create PR +git push -u origin HEAD +gh pr create \ + --title "Bump cli-compat to appkit {appkit_version}, skills {skills_version}" \ + --body "$(cat <<'EOF' +## Summary +Bump `cli-compat.json` to use: +- AppKit `{appkit_version}` +- Agent Skills `{skills_version}` + +## Checklist +- [ ] Evals passed with no regressions +- [ ] `go test ./libs/clicompat/... -run TestEmbeddedManifest` passes +EOF +)" +``` + +Show the PR URL to the user when done. + +## Examples + +### Example: With explicit versions +``` +/bump-cli-compat --appkit 0.28.0 --skills 0.1.6 +``` +Validates tags exist (including `template-v0.28.0`), updates manifest, creates PR. + +### Example: Auto-detect latest +``` +/bump-cli-compat +``` +Fetches latest tags, asks for eval confirmation, then updates and creates PR. + +### Example: Only bump AppKit +``` +/bump-cli-compat --appkit 0.28.0 +``` +Auto-detects latest skills version, asks for confirmation, then updates both. diff --git a/.github/OWNERS b/.github/OWNERS index 7cae525465a..579fcc2d44b 100644 --- a/.github/OWNERS +++ b/.github/OWNERS @@ -59,5 +59,9 @@ # Internal /internal/ team:platform +# CLI compatibility manifest +/internal/build/cli-compat.json team:eng-apps-devex team:platform +/libs/clicompat/ team:eng-apps-devex team:platform + # Experimental /experimental/aitools/ team:eng-apps-devex @lennartkats-db diff --git a/cmd/apps/init.go b/cmd/apps/init.go index 2f1da99bb71..35eb45cd99e 100644 --- a/cmd/apps/init.go +++ b/cmd/apps/init.go @@ -23,6 +23,7 @@ import ( "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/clicompat" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/env" @@ -38,7 +39,6 @@ const ( appkitTemplateDir = "template" appkitDefaultBranch = "main" appkitTemplateTagPfx = "template-v" - appkitDefaultVersion = "template-v0.24.0" defaultProfile = "DEFAULT" ) @@ -169,7 +169,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", "", fmt.Sprintf("AppKit version to use (default: %s, use 'latest' for main branch)", appkitDefaultVersion)) + versionDesc := "AppKit version to use (use 'latest' for main branch)" + if v, err := clicompat.ResolveEmbeddedAppKitVersion(); err == nil && 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") @@ -805,8 +809,12 @@ func runCreate(ctx context.Context, opts createOptions) error { case opts.version != "": gitRef = normalizeVersion(opts.version) default: - // Default: use pinned version - gitRef = appkitDefaultVersion + appkitVersion, err := clicompat.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(appkitVersion) + cmdio.LogString(ctx, "Using AppKit template version "+appkitVersion) } templateSrc = appkitRepoURL } @@ -859,6 +867,17 @@ func runCreate(ctx context.Context, opts createOptions) error { // Step 2: Wait for template (may already be done if the user took time typing the name) resolvedPath, cleanup, err := awaitTemplate(ctx, templateCh) + if err != nil && usingDefaultTemplate && clicompat.IsNotFoundError(err) { + // The resolved version doesn't exist as a tag. Fall back to the + // embedded manifest which ships a known-good version. + fallbackVersion, fbErr := clicompat.ResolveEmbeddedAppKitVersion() + if fbErr == nil && fallbackVersion != "" { + log.Warnf(ctx, "Template version not found, falling back to embedded version %s", fallbackVersion) + fallbackRef := normalizeVersion(fallbackVersion) + templateCh = resolveTemplateAsync(ctx, templateSrc, fallbackRef, appkitTemplateDir) + resolvedPath, cleanup, err = awaitTemplate(ctx, templateCh) + } + } if err != nil { return err } diff --git a/cmd/apps/init_test.go b/cmd/apps/init_test.go index 81e71eed3fb..fe406dfb7b3 100644 --- a/cmd/apps/init_test.go +++ b/cmd/apps/init_test.go @@ -443,7 +443,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 38df201acc0..da904b14715 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/clicompat" "github.com/databricks/cli/libs/env" "github.com/spf13/cobra" ) @@ -27,7 +28,11 @@ func runManifestOnly(ctx context.Context, templatePath, branch, version string) case version != "": gitRef = normalizeVersion(version) default: - gitRef = appkitDefaultVersion + appkitVersion, err := clicompat.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(appkitVersion) } templateSrc = appkitRepoURL } diff --git a/experimental/aitools/cmd/list.go b/experimental/aitools/cmd/list.go index 1be1538c9a0..8f665d7ad12 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 67c38fec42a..a5bde04e95d 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:") @@ -80,6 +84,12 @@ func printVersionLine(ctx context.Context, label string, state *installer.Instal skillNoun = "skill" } + if latestRef == "" { + cmdio.LogString(ctx, fmt.Sprintf(" %s: v%s (%d %s)", label, version, len(state.Skills), skillNoun)) + cmdio.LogString(ctx, " Last updated: "+state.LastUpdated.Format("2006-01-02")) + return + } + if latestRef == state.Release { cmdio.LogString(ctx, fmt.Sprintf(" %s: v%s (%d %s, up to date)", label, version, len(state.Skills), skillNoun)) cmdio.LogString(ctx, " Last updated: "+state.LastUpdated.Format("2006-01-02")) diff --git a/experimental/aitools/lib/installer/SKILLS_VERSION b/experimental/aitools/lib/installer/SKILLS_VERSION deleted file mode 100644 index 027a383a35e..00000000000 --- 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 53285f6ffc9..8c351e48ec6 100644 --- a/experimental/aitools/lib/installer/installer.go +++ b/experimental/aitools/lib/installer/installer.go @@ -16,6 +16,7 @@ import ( "github.com/databricks/cli/experimental/aitools/lib/agents" "github.com/databricks/cli/internal/build" + "github.com/databricks/cli/libs/clicompat" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/log" @@ -32,13 +33,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 := clicompat.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. @@ -92,8 +97,22 @@ 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 + } + tag := strings.TrimPrefix(ref, "v") + cmdio.LogString(ctx, "Using skills version "+tag) manifest, err := src.FetchManifest(ctx, ref) + if err != nil && clicompat.IsNotFoundError(err) { + // The resolved version doesn't exist. Fall back to the embedded manifest. + fallbackVersion, fbErr := clicompat.ResolveEmbeddedAgentSkillsVersion() + if fbErr == nil && fallbackVersion != "" && fallbackVersion != tag { + log.Warnf(ctx, "Skills version %s not found, falling back to embedded version %s", tag, fallbackVersion) + ref = "v" + fallbackVersion + manifest, err = src.FetchManifest(ctx, ref) + } + } if err != nil { return err } @@ -193,12 +212,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 b769143906d..848ab54172c 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" @@ -19,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 @@ -192,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) @@ -204,18 +205,19 @@ 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"]) - 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) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) @@ -232,13 +234,14 @@ 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) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) src := &mockManifestSource{manifest: testManifest()} agent := testAgent(tmp) @@ -254,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{ @@ -275,13 +279,14 @@ 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) { tmp := setupTestHome(t) ctx, stderr := cmdio.NewTestContextWithStderr(t.Context()) setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) manifest := testManifest() manifest.Skills["databricks-experimental"] = SkillMeta{ @@ -305,13 +310,14 @@ 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) { 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. @@ -339,7 +345,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") } @@ -347,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() @@ -371,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) @@ -400,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) @@ -437,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"]) } @@ -445,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") @@ -463,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") @@ -481,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) @@ -526,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") @@ -546,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") @@ -590,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") @@ -607,17 +622,17 @@ 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) - 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) { 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)) @@ -660,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)) @@ -680,13 +696,14 @@ 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) { 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)) @@ -725,3 +742,19 @@ func TestSupportsProjectScopeSetCorrectly(t *testing.T) { assert.Equal(t, want, agent.SupportsProjectScope, "SupportsProjectScope for %s", agent.Name) } } + +func TestGetSkillsRefResolvesFromManifest(t *testing.T) { + // Pre-populate the cache so FetchManifest returns from tier 1 (local cache) + // without hitting the network. The embedded manifest fallback is tested + // separately in clicompat_test.go. + cacheDir := t.TempDir() + t.Setenv("DATABRICKS_CACHE_DIR", cacheDir) + cachePath := filepath.Join(cacheDir, "compat-manifest.json") + manifest := `{"next":{"appkit":"0.24.0","skills":"0.1.5"},"0.300.0":{"appkit":"0.24.0","skills":"0.1.5"}}` + require.NoError(t, os.WriteFile(cachePath, []byte(manifest), 0o644)) + + ref, err := GetSkillsRef(t.Context()) + require.NoError(t, err, "GetSkillsRef should succeed via cached manifest") + assert.NotEmpty(t, ref) + assert.True(t, len(ref) > 1 && ref[0] == 'v', "ref should start with 'v', got %q", ref) +} diff --git a/experimental/aitools/lib/installer/uninstall_test.go b/experimental/aitools/lib/installer/uninstall_test.go index 6c7589f6f29..444fb773af7 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 663ad5e908e..0aa260f115a 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 97e3014be65..3272a77babe 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 0e942ca0a95..00000000000 --- 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) diff --git a/internal/build/README.md b/internal/build/README.md new file mode 100644 index 00000000000..48ddcad1523 --- /dev/null +++ b/internal/build/README.md @@ -0,0 +1,55 @@ +# CLI Compatibility Manifest + +`cli-compat.json` maps Databricks CLI versions to compatible AppKit and Agent Skills versions. The CLI uses this manifest to determine which template version to use for `apps init` and which skills version to use for `aitools install`. + +## Manifest format + +```json +{ + "next": { "appkit": "0.24.0", "skills": "0.1.5" }, + "0.300.0": { "appkit": "0.24.0", "skills": "0.1.5" } +} +``` + +- Each key is a CLI version (`X.Y.Z`) or `"next"`. +- Each value specifies the compatible `appkit` and `skills` versions. +- `"next"` is used for dev builds (`0.0.0-dev*`). For production CLI versions newer than all listed entries, the highest versioned entry is used. + +## How the CLI resolves versions + +1. **Exact match** on CLI version → use that entry. +2. **No exact match**, between two entries → use the nearest lower version's entry. +3. **Newer than all entries** → use the highest versioned entry. +4. **Older than all entries** → use the lowest (oldest) entry. +5. **Dev builds** (`0.0.0-dev*`) → use `"next"`. + +## Manifest sources (fallback chain) + +At runtime, the CLI resolves the manifest from four sources: + +1. **Local cache** (`~/.cache/databricks/compat-manifest.json`) — used if fresh (< 1 hour old). +2. **Remote fetch** from GitHub — used when cache is stale or missing. On success, the local cache is updated. +3. **Stale local cache** — if remote fetch fails but a previously cached file exists (even if expired), it is used as-is. +4. **Embedded manifest** — compiled into the binary via `go:embed`. Used as last resort when both remote and local cache fail. + +## When to update + +After each AppKit or Agent Skills release: + +1. **Run evals** on the new AppKit version. If there is no regression, proceed. +2. **Open a PR** to update `cli-compat.json`. The change depends on the type of release: + - **No template changes** (just an AppKit/skills version bump): search & replace all version occurrences in the manifest and update `next`. + - **Template changes that don't require new CLI features**: test the last 3 CLI versions with the new template and update matching entries. + - **Template changes that require new CLI features**: add a new entry for the minimum CLI version that supports them; older entries keep pointing to the previous template version. + +This process is manual for now but can be automated as part of the release workflow in the future. Use the `/bump-cli-compat` Claude Code skill to automate the update and PR creation. + +## Validation + +The manifest is validated by Go tests in `libs/clicompat/`: + +```bash +go test ./libs/clicompat/... -run TestEmbeddedManifest -v +``` + +This checks: valid JSON, `"next"` key present, at least one versioned entry, valid semver keys, `next` versions >= all entries, and ascending key order. diff --git a/internal/build/cli-compat.json b/internal/build/cli-compat.json new file mode 100644 index 00000000000..33fc41f0a64 --- /dev/null +++ b/internal/build/cli-compat.json @@ -0,0 +1,4 @@ +{ + "next": { "appkit": "0.24.0", "skills": "0.1.5" }, + "0.300.0": { "appkit": "0.24.0", "skills": "0.1.5" } +} diff --git a/internal/build/clicompat.go b/internal/build/clicompat.go new file mode 100644 index 00000000000..22cb8f13a03 --- /dev/null +++ b/internal/build/clicompat.go @@ -0,0 +1,9 @@ +package build + +import _ "embed" + +// CLICompatManifestJSON is the cli-compat.json manifest embedded at compile time. +// Used as the last-resort fallback when both remote fetch and local cache fail. +// +//go:embed cli-compat.json +var CLICompatManifestJSON []byte diff --git a/libs/clicompat/clicompat.go b/libs/clicompat/clicompat.go new file mode 100644 index 00000000000..e77ec7da9c2 --- /dev/null +++ b/libs/clicompat/clicompat.go @@ -0,0 +1,376 @@ +package clicompat + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "slices" + "strings" + "time" + + "github.com/databricks/cli/internal/build" + "github.com/databricks/cli/libs/env" + "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/cli/main/internal/build/cli-compat.json" + + // fetchTimeout is the HTTP timeout for fetching the manifest at runtime. + fetchTimeout = 3 * time.Second + + // nextKey is the special manifest key for CLI versions newer than any entry. + nextKey = "next" + + // cacheTTL is how long a locally cached manifest is considered fresh. + cacheTTL = 1 * time.Hour + + // localManifestFile is the filename for the locally cached manifest. + localManifestFile = "compat-manifest.json" + + // devVersionPrefix identifies dev builds that always use the "next" entry. + devVersionPrefix = "0.0.0-dev" + + fetchRetries = 2 + fetchRetryBackoff = 300 * time.Millisecond +) + +// Entry maps a CLI version to compatible AppKit and Agent Skills versions. +type Entry struct { + AppKit string `json:"appkit"` + AgentSkills string `json:"skills"` +} + +// Manifest is the compatibility manifest: a map of CLI version strings to entries. +type Manifest map[string]Entry + +// cachedManifest holds a parsed manifest together with its on-disk mod time. +type cachedManifest struct { + manifest Manifest + modTime time.Time +} + +// isFresh reports whether the cached manifest is younger than maxAge. +func (c cachedManifest) isFresh(maxAge time.Duration) bool { + return time.Since(c.modTime) < maxAge +} + +// 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 using a 4-tier fallback: +// 1. Local cached file (if fresh, < 1 hour old) +// 2. Remote fetch from GitHub (with retry) +// 3. Stale local file (if remote fails but a previously cached file exists) +// 4. Embedded manifest compiled into the binary +func FetchManifest(ctx context.Context) (Manifest, error) { + localPath := manifestLocalPath(ctx) + + // Read local file once — reuse across tiers. + local, localErr := readLocalManifest(localPath) + + // Tier 1: local file is fresh. + if localErr == nil && local.isFresh(cacheTTL) { + log.Debugf(ctx, "Using cached manifest from %s", localPath) + return local.manifest, nil + } + + // Tier 2: fetch from remote (local file missing or stale). + m, fetchErr := fetchRemoteWithRetry(ctx) + if fetchErr == nil { + writeLocalManifest(ctx, localPath, m) + return m, nil + } + + // Tier 3a: local file exists but stale — use it anyway. + if localErr == nil { + log.Debugf(ctx, "Using stale cached manifest (remote failed: %v)", fetchErr) + return local.manifest, nil + } + + // Tier 3b: embedded manifest. + m, embeddedErr := parseManifest(build.CLICompatManifestJSON) + if embeddedErr == nil { + log.Debugf(ctx, "Using embedded manifest (remote and local cache failed)") + return m, nil + } + + return nil, fmt.Errorf("all manifest sources failed (remote: %w, embedded: %w)", fetchErr, embeddedErr) +} + +// ResolveEmbeddedAppKitVersion resolves the AppKit version from only the +// embedded manifest for the current CLI version. Used as a fallback when the +// primary version (from remote or cached manifest) points to a non-existent tag, +// and for help text defaults where a network call is not appropriate. +func ResolveEmbeddedAppKitVersion() (string, error) { + m, err := parseManifest(build.CLICompatManifestJSON) + if err != nil { + return "", fmt.Errorf("embedded manifest: %w", err) + } + entry, err := Resolve(m, build.GetInfo().Version) + if err != nil { + return "", fmt.Errorf("embedded manifest resolve: %w", err) + } + return entry.AppKit, nil +} + +// ResolveEmbeddedAgentSkillsVersion resolves the Agent Skills version from only +// the embedded manifest for the current CLI version. Used as a fallback when the +// primary version points to a non-existent tag. +func ResolveEmbeddedAgentSkillsVersion() (string, error) { + m, err := parseManifest(build.CLICompatManifestJSON) + if err != nil { + return "", fmt.Errorf("embedded manifest: %w", err) + } + entry, err := Resolve(m, build.GetInfo().Version) + if err != nil { + return "", fmt.Errorf("embedded manifest resolve: %w", err) + } + return entry.AgentSkills, nil +} + +// IsNotFoundError reports whether the error indicates a "not found" condition +// (e.g. HTTP 404, missing git branch/tag). Used by consumers to decide whether +// to fall back to the embedded manifest. +func IsNotFoundError(err error) bool { + if err == nil { + return false + } + msg := strings.ToLower(err.Error()) + return strings.Contains(msg, "not found") || strings.Contains(msg, "404") +} + +// 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). This also handles CLI versions +// newer than all entries, returning the highest known entry. +// 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{}, errors.New("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, devVersionPrefix) { + 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. + slices.SortFunc(versions, func(a, b string) int { + return semver.Compare("v"+b, "v"+a) + }) + + // 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 — 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. +func resolveEntry(ctx context.Context, cliVersion string) (Entry, error) { + m, err := FetchManifest(ctx) + if err != nil { + return Entry{}, err + } + return Resolve(m, cliVersion) +} + +// 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.AgentSkills, nil +} + +// --- Local manifest cache --- + +// manifestLocalPath returns the path to the locally cached manifest file. +func manifestLocalPath(ctx context.Context) string { + if dir := env.Get(ctx, "DATABRICKS_CACHE_DIR"); dir != "" { + return filepath.Join(dir, localManifestFile) + } + home, err := os.UserCacheDir() + if err != nil { + return "" + } + return filepath.Join(home, "databricks", localManifestFile) +} + +// readLocalManifest reads and parses the locally cached manifest file. +func readLocalManifest(path string) (cachedManifest, error) { + if path == "" { + return cachedManifest{}, errors.New("no cache path") + } + info, err := os.Stat(path) + if err != nil { + return cachedManifest{}, err + } + data, err := os.ReadFile(path) + if err != nil { + return cachedManifest{}, err + } + m, err := parseManifest(data) + if err != nil { + return cachedManifest{}, err + } + return cachedManifest{manifest: m, modTime: info.ModTime()}, nil +} + +// writeLocalManifest writes the manifest to the local cache path using a +// temp-file-then-rename pattern. The os.Remove before os.Rename is needed +// for Windows, where Rename fails if the destination exists. +func writeLocalManifest(ctx context.Context, path string, m Manifest) { + if path == "" { + return + } + data, err := json.Marshal(m) + if err != nil { + log.Debugf(ctx, "Failed to marshal manifest for cache: %v", err) + return + } + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + log.Debugf(ctx, "Failed to create cache directory %s: %v", dir, err) + return + } + tmp, err := os.CreateTemp(dir, ".compat-manifest-*.tmp") + if err != nil { + log.Debugf(ctx, "Failed to create temp cache file: %v", err) + return + } + tmpPath := tmp.Name() + defer func() { + _ = tmp.Close() + _ = os.Remove(tmpPath) + }() + if _, err := tmp.Write(data); err != nil { + log.Debugf(ctx, "Failed to write temp cache file: %v", err) + return + } + if err := tmp.Close(); err != nil { + log.Debugf(ctx, "Failed to close temp cache file: %v", err) + return + } + _ = os.Remove(path) + if err := os.Rename(tmpPath, path); err != nil { + log.Debugf(ctx, "Failed to rename temp cache file: %v", err) + } +} + +// --- Remote fetch --- + +// 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 + } + + 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) + } + + // 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 + } + + 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, errors.New("empty compatibility manifest") + } + 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/clicompat/clicompat_test.go b/libs/clicompat/clicompat_test.go new file mode 100644 index 00000000000..f40f64dd806 --- /dev/null +++ b/libs/clicompat/clicompat_test.go @@ -0,0 +1,433 @@ +package clicompat + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "slices" + "sync/atomic" + "testing" + "time" + + "github.com/databricks/cli/internal/build" + "github.com/databricks/cli/libs/env" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/mod/semver" +) + +// 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 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 + 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 }) +} + +// 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", 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"}, + } +} + +// --- Resolve tests (unchanged, no network) --- + +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.AgentSkills) +} + +func TestResolve_NearestLower(t *testing.T) { + m := testManifest() + 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.AgentSkills) +} + +func TestResolve_NewerThanAll(t *testing.T) { + m := Manifest{ + "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) + assert.Equal(t, "0.27.0", entry.AppKit) + assert.Equal(t, "0.1.5", entry.AgentSkills) +} + +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.AgentSkills) +} + +func TestResolve_OlderThanAll(t *testing.T) { + m := testManifest() + entry, err := Resolve(m, "0.280.0") + require.NoError(t, err) + assert.Equal(t, "0.24.0", entry.AppKit) + assert.Equal(t, "0.1.4", entry.AgentSkills) +} + +func TestResolve_OnlyNextKey(t *testing.T) { + m := Manifest{ + "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.AgentSkills) +} + +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.AgentSkills) +} + +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", AgentSkills: "0.1.5"}, + } + _, err := Resolve(m, "0.296.0") + assert.Error(t, err) + assert.Contains(t, err.Error(), `missing "next" key`) +} + +// --- FetchManifest tests --- + +func TestFetchManifest_RemoteSuccess(t *testing.T) { + ctx := testContext(t) + want := Manifest{ + "next": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", AgentSkills: "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) + + 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) +} + +func TestFetchManifest_RemoteFailFallsBackToEmbedded(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) + + // No local cache exists, so should fall back to embedded manifest. + result, err := FetchManifest(ctx) + require.NoError(t, err) + + // Verify it returned the embedded manifest values. + embedded, _ := parseManifest(build.CLICompatManifestJSON) + assert.Equal(t, embedded["next"].AppKit, result["next"].AppKit) +} + +func TestFetchManifest_RemoteFailFallsBackToStaleCache(t *testing.T) { + ctx := testContext(t) + + // Pre-populate the local cache with a stale manifest. + staleManifest := Manifest{ + "next": {AppKit: "0.88.0", AgentSkills: "0.8.8"}, + "0.296.0": {AppKit: "0.88.0", AgentSkills: "0.8.8"}, + } + localPath := manifestLocalPath(ctx) + writeLocalManifest(ctx, localPath, staleManifest) + // Make it stale by setting mod time to 2 hours ago. + past := time.Now().Add(-2 * time.Hour) + require.NoError(t, os.Chtimes(localPath, past, past)) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + redirectToServer(t, srv) + + result, err := FetchManifest(ctx) + require.NoError(t, err) + // Should return the stale cached manifest, not the embedded one. + assert.Equal(t, "0.88.0", result["next"].AppKit) +} + +func TestFetchManifest_RemoteSuccessWritesLocalCache(t *testing.T) { + ctx := testContext(t) + want := Manifest{ + "next": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", AgentSkills: "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) + + _, err := FetchManifest(ctx) + require.NoError(t, err) + + // Verify the local cache was written. + localPath := manifestLocalPath(ctx) + _, statErr := os.Stat(localPath) + assert.NoError(t, statErr, "local cache file should exist after successful fetch") +} + +func TestFetchManifest_CacheHit(t *testing.T) { + ctx := testContext(t) + want := Manifest{ + "next": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", AgentSkills: "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", AgentSkills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", AgentSkills: "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) + assert.Equal(t, "0.99.0", result["next"].AppKit) + assert.Equal(t, int32(2), callCount.Load(), "should have retried after first failure") +} + +// --- parseManifest tests --- + +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") +} + +// --- resolveEntry tests --- + +func TestResolveEntry_RemoteSuccess(t *testing.T) { + ctx := testContext(t) + want := Manifest{ + "next": {AppKit: "0.99.0", AgentSkills: "0.9.9"}, + "0.296.0": {AppKit: "0.99.0", AgentSkills: "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.AgentSkills) +} + +func TestResolveEntry_RemoteFailUsesEmbedded(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) + + // Should succeed via the embedded manifest fallback. + entry, err := resolveEntry(ctx, "0.0.0-dev+test") + require.NoError(t, err) + assert.NotEmpty(t, entry.AppKit) +} + +// --- ResolveEmbeddedAppKitVersion --- + +func TestResolveEmbeddedAppKitVersion(t *testing.T) { + v, err := ResolveEmbeddedAppKitVersion() + require.NoError(t, err) + assert.NotEmpty(t, v, "embedded manifest should resolve an appkit version") + assert.True(t, semver.IsValid("v"+v), "embedded resolved version should be valid semver") +} + +// --- Embedded manifest validation (replaces AppKit TS validator) --- + +func TestEmbeddedManifest_IsWellFormed(t *testing.T) { + m, err := parseManifest(build.CLICompatManifestJSON) + require.NoError(t, err, "embedded manifest must be valid JSON") + + // Must have "next" key. + next, ok := m[nextKey] + require.True(t, ok, "embedded manifest must have %q key", nextKey) + assert.NotEmpty(t, next.AppKit, "next.appkit must be set") + assert.NotEmpty(t, next.AgentSkills, "next.skills must be set") + + // Must have at least one versioned entry. + var versionedKeys []string + for k := range m { + if k != nextKey { + versionedKeys = append(versionedKeys, k) + } + } + assert.NotEmpty(t, versionedKeys, "must have at least one versioned CLI entry besides %q", nextKey) + + // All versioned keys must be valid semver. + for _, k := range versionedKeys { + assert.True(t, semver.IsValid("v"+k), "key %q must be valid semver", k) + } + + // Sort to get deterministic order from map iteration. + slices.SortFunc(versionedKeys, func(a, b string) int { + return semver.Compare("v"+a, "v"+b) + }) + + // "next" versions must be >= all versioned entries. + for _, k := range versionedKeys { + entry := m[k] + assert.GreaterOrEqual(t, semver.Compare("v"+next.AppKit, "v"+entry.AppKit), 0, + "next.appkit (%s) must be >= %s.appkit (%s)", next.AppKit, k, entry.AppKit) + assert.GreaterOrEqual(t, semver.Compare("v"+next.AgentSkills, "v"+entry.AgentSkills), 0, + "next.skills (%s) must be >= %s.skills (%s)", next.AgentSkills, k, entry.AgentSkills) + } + + // Versioned keys must be in ascending semver order. + for i := 1; i < len(versionedKeys); i++ { + cmp := semver.Compare("v"+versionedKeys[i-1], "v"+versionedKeys[i]) + assert.LessOrEqual(t, cmp, 0, + "versioned keys must be in ascending order: %s should come before %s", + versionedKeys[i-1], versionedKeys[i]) + } +} + +// --- Local cache helpers --- + +func TestManifestLocalPath(t *testing.T) { + ctx := env.Set(t.Context(), "DATABRICKS_CACHE_DIR", "/tmp/test-cache") + path := manifestLocalPath(ctx) + assert.Equal(t, filepath.Join("/tmp/test-cache", localManifestFile), path) +} + +func TestReadWriteLocalManifest(t *testing.T) { + ctx := testContext(t) + m := Manifest{ + "next": {AppKit: "0.50.0", AgentSkills: "0.5.0"}, + "0.300.0": {AppKit: "0.50.0", AgentSkills: "0.5.0"}, + } + + path := manifestLocalPath(ctx) + writeLocalManifest(ctx, path, m) + + cached, err := readLocalManifest(path) + require.NoError(t, err) + assert.Equal(t, "0.50.0", cached.manifest["next"].AppKit) + assert.True(t, cached.isFresh(cacheTTL), "just-written file should be fresh") +}