diff --git a/.agent/skills/bump-cli-compat/SKILL.md b/.agent/skills/bump-cli-compat/SKILL.md new file mode 100644 index 0000000000..a0612a2ba0 --- /dev/null +++ b/.agent/skills/bump-cli-compat/SKILL.md @@ -0,0 +1,153 @@ +--- +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: Determine update type + +Ask the user: + +> Do any of these apply? +> - **AppKit**: The new templates require new CLI logic in `apps init` (e.g. new flags, prompts, or template handling that older CLIs don't have) +> - **Skills**: The new skills version uses CLI commands that older CLIs don't support +> +> If **neither** applies, this is a non-breaking bump (default). + +- **No breaking changes** (default): proceed to Step 4a. +- **Breaking changes**: proceed to Step 4b. + +### Step 4a: No breaking changes (update in-place) + +Update the **highest versioned entry** to the new appkit and skills versions. Do NOT add new versioned keys. The manifest is range-based: updating the highest entry automatically covers all CLI versions in that range. + +Write the updated `internal/build/cli-compat.json`. + +### Step 4b: Breaking changes (add new entry) + +Ask the user for the **minimum CLI version** that supports the new features. + +Add a new entry keyed to that CLI version with the new appkit and skills versions. Keep older entries unchanged so older CLI binaries stay compatible. + +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 7cae525465..579fcc2d44 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 2f1da99bb7..abbe40828e 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,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 (default: auto-detected, 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") @@ -805,8 +805,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 +863,20 @@ 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) + // Only fall back to the embedded version when the version was auto-resolved + // from the manifest, not when the user explicitly passed --version or --branch. + versionAutoResolved := opts.version == "" && opts.branch == "" + if err != nil && usingDefaultTemplate && versionAutoResolved && clicompat.IsNotFoundError(err) { + fallbackVersion, fbErr := clicompat.ResolveEmbeddedAppKitVersion() + if fbErr == nil && fallbackVersion != "" && normalizeVersion(fallbackVersion) != branchForClone { + 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) + } else if fbErr != nil { + log.Warnf(ctx, "Could not resolve embedded AppKit version: %v", fbErr) + } + } if err != nil { return err } diff --git a/cmd/apps/init_test.go b/cmd/apps/init_test.go index 81e71eed3f..fe406dfb7b 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 38df201acc..2fcc29099f 100644 --- a/cmd/apps/manifest.go +++ b/cmd/apps/manifest.go @@ -9,7 +9,9 @@ import ( "path/filepath" "github.com/databricks/cli/libs/apps/manifest" + "github.com/databricks/cli/libs/clicompat" "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" "github.com/spf13/cobra" ) @@ -27,7 +29,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 } @@ -39,6 +45,17 @@ func runManifestOnly(ctx context.Context, templatePath, branch, version string) subdirForClone = appkitTemplateDir } resolvedPath, cleanup, err := resolveTemplate(ctx, templateSrc, branchForClone, subdirForClone) + versionAutoResolved := version == "" && branch == "" + if err != nil && usingDefaultTemplate && versionAutoResolved && clicompat.IsNotFoundError(err) { + fallbackVersion, fbErr := clicompat.ResolveEmbeddedAppKitVersion() + if fbErr == nil && fallbackVersion != "" && normalizeVersion(fallbackVersion) != gitRef { + log.Warnf(ctx, "Template version not found, falling back to embedded version %s", fallbackVersion) + fallbackRef := normalizeVersion(fallbackVersion) + resolvedPath, cleanup, err = resolveTemplate(ctx, templateSrc, fallbackRef, appkitTemplateDir) + } else if fbErr != nil { + log.Warnf(ctx, "Could not resolve embedded AppKit version: %v", fbErr) + } + } if err != nil { return err } diff --git a/experimental/aitools/cmd/list.go b/experimental/aitools/cmd/list.go index 1be1538c9a..90d397aba6 100644 --- a/experimental/aitools/cmd/list.go +++ b/experimental/aitools/cmd/list.go @@ -48,10 +48,13 @@ func newListCmd() *cobra.Command { func defaultListSkills(cmd *cobra.Command, scope string) error { ctx := cmd.Context() - ref := installer.GetSkillsRef(ctx) + ref, explicit, err := installer.GetSkillsRef(ctx) + if err != nil { + return err + } src := &installer.GitHubManifestSource{} - manifest, err := src.FetchManifest(ctx, ref) + manifest, ref, err := installer.FetchSkillsManifestWithFallback(ctx, src, ref, !explicit) if err != nil { return fmt.Errorf("failed to fetch manifest: %w", err) } diff --git a/experimental/aitools/cmd/version.go b/experimental/aitools/cmd/version.go index 67c38fec42..fd3d086e95 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 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 53285f6ffc..dbf511720b 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,18 @@ 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 and whether it was explicitly +// set via DATABRICKS_SKILLS_REF (as opposed to auto-resolved from the manifest). +// Resolution order: DATABRICKS_SKILLS_REF env var → compatibility manifest → error. +func GetSkillsRef(ctx context.Context) (ref string, explicit bool, err error) { if ref := env.Get(ctx, "DATABRICKS_SKILLS_REF"); ref != "" { - return ref + return ref, true, nil } - return defaultSkillsRepoRef + v, err := clicompat.ResolveAgentSkillsVersion(ctx) + if err != nil { + return "", false, fmt.Errorf("could not resolve skills version: %w", err) + } + return "v" + v, false, nil } // Manifest describes the skills manifest fetched from the skills repo. @@ -88,12 +94,36 @@ func fetchSkillFile(ctx context.Context, ref, skillName, filePath string) ([]byt return io.ReadAll(resp.Body) } +// FetchSkillsManifestWithFallback fetches the skills manifest at the given ref. +// If the ref points to a non-existent tag (not-found error), it falls back to +// the embedded manifest's skills version. Returns the manifest, the (possibly +// updated) ref, and any error. +func FetchSkillsManifestWithFallback(ctx context.Context, src ManifestSource, ref string, allowFallback bool) (*Manifest, string, error) { + tag := strings.TrimPrefix(ref, "v") + manifest, err := src.FetchManifest(ctx, ref) + if err != nil && allowFallback && clicompat.IsNotFoundError(err) { + 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) + } else if fbErr != nil { + log.Warnf(ctx, "Could not resolve embedded skills version: %v", fbErr) + } + } + return manifest, ref, err +} + // InstallSkillsForAgents fetches the manifest and installs skills for the given agents. // This is the core installation function. Callers are responsible for agent detection, // prompting, and printing the "Installing..." header. func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgents []*agents.Agent, opts InstallOptions) error { - ref := GetSkillsRef(ctx) - manifest, err := src.FetchManifest(ctx, ref) + ref, explicit, err := GetSkillsRef(ctx) + if err != nil { + return err + } + cmdio.LogString(ctx, "Using skills version "+strings.TrimPrefix(ref, "v")) + manifest, ref, err := FetchSkillsManifestWithFallback(ctx, src, ref, !explicit) if err != nil { return err } @@ -193,12 +223,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..ab64f8605b 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,20 @@ 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, explicit, err := GetSkillsRef(t.Context()) + require.NoError(t, err, "GetSkillsRef should succeed via cached manifest") + assert.False(t, explicit, "ref resolved from manifest should not be explicit") + 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/source.go b/experimental/aitools/lib/installer/source.go index e601b26d66..5dc78a254f 100644 --- a/experimental/aitools/lib/installer/source.go +++ b/experimental/aitools/lib/installer/source.go @@ -7,6 +7,7 @@ import ( "net/http" "time" + "github.com/databricks/cli/libs/clicompat" "github.com/databricks/cli/libs/log" ) @@ -37,6 +38,9 @@ func (s *GitHubManifestSource) FetchManifest(ctx context.Context, ref string) (* } defer resp.Body.Close() + if resp.StatusCode == http.StatusNotFound { + return nil, fmt.Errorf("skills manifest at %s@%s: %w", skillsRepoName, ref, clicompat.ErrNotFound) + } if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("failed to fetch manifest: HTTP %d", resp.StatusCode) } 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..720359d334 100644 --- a/experimental/aitools/lib/installer/update.go +++ b/experimental/aitools/lib/installer/update.go @@ -81,14 +81,17 @@ 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, explicit, err := GetSkillsRef(ctx) + if err != nil { + return nil, err + } if state.Release == latestTag && !opts.Force { cmdio.LogString(ctx, "Already up to date.") return &UpdateResult{Unchanged: slices.Sorted(maps.Keys(state.Skills))}, nil } - manifest, err := src.FetchManifest(ctx, latestTag) + manifest, latestTag, err := FetchSkillsManifestWithFallback(ctx, src, latestTag, !explicit) if err != nil { if opts.Check { log.Warnf(ctx, "Could not fetch manifest: %v", err) 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) diff --git a/internal/build/README.md b/internal/build/README.md new file mode 100644 index 0000000000..913ae7e5f4 --- /dev/null +++ b/internal/build/README.md @@ -0,0 +1,69 @@ +# 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 +{ + "0.296.0": { "appkit": "0.27.0", "skills": "0.1.5" }, + "0.290.0": { "appkit": "0.24.0", "skills": "0.1.4" }, + "0.280.0": { "appkit": "0.20.0", "skills": "0.1.0" } +} +``` + +Each key is a CLI version in semver format. Each entry defines a **range floor**: it applies to that CLI version and all versions above it, up to (but not including) the next entry. The manifest should be **sparse** — not every CLI version needs its own entry. Only add a new entry when a compatibility boundary changes. + +For example, with the manifest above: +- CLI `0.285.0` → uses `0.280.0` entry (appkit `0.20.0`) +- CLI `0.293.0` → uses `0.290.0` entry (appkit `0.24.0`) +- CLI `0.300.0` → uses `0.296.0` entry (appkit `0.27.0`, highest versioned) +- CLI `0.0.0-dev+abc` → uses `0.296.0` entry (dev builds use the highest versioned entry) + +## How the CLI resolves versions + +1. **Dev builds** (`0.0.0-dev*`) → use the highest versioned entry. +2. **Exact match** on CLI version → use that entry. +3. **No exact match**, between two entries → use the nearest lower version's entry. +4. **Newer than all entries** → use the highest versioned entry. +5. **Older than all entries** → use the lowest (oldest) entry. + +## 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. + +Set `DATABRICKS_FORCE_EMBEDDED_COMPAT=true` to skip all tiers and use only the embedded manifest. This is useful for local development when testing with a locally compiled binary that has a modified `cli-compat.json`. + +## When to update + +The goal is to **keep the manifest sparse** — only add entries at compatibility boundaries. 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 breaking changes** (the new AppKit/skills version works with all existing CLI versions): update the existing highest versioned entry's appkit/skills values in-place. Do NOT add a new versioned key. All CLI versions in that range automatically pick up the new versions. + - **Breaking changes** (the new AppKit templates require specific `apps init` features, or the new skills version requires CLI commands that older CLIs lack): add a new entry keyed to the **minimum CLI version** that supports the new features. Older entries keep their previous appkit/skills values so older CLI binaries stay compatible. + +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, at least one entry, valid semver keys, valid semver entry values, and ascending key order. + +## Pruning policy + +Entries MUST NOT be removed from the manifest. Older CLI binaries use the lowest entry as their floor when the CLI version is older than all entries. Pruning it causes them to silently resolve to a newer entry that may require CLI features they lack. If the manifest grows too large, consider archiving very old entries to a separate file while keeping the oldest entry as a sentinel. + +## Trust model + +The live manifest is fetched over HTTPS from GitHub (`raw.githubusercontent.com`). The trust boundary is: TLS certificate validation + GitHub's access controls + write access to the `main` branch of `databricks/cli`. A compromised manifest can only steer clients to existing published tags (AppKit or skills); it cannot inject arbitrary code. The CLI binary always ships an embedded fallback manifest that limits exposure to cache-TTL windows (1 hour). The local cache (`~/.cache/databricks/compat-manifest.json`) is trust-on-disk: an attacker with user-level write access to the cache directory could swap in a malicious manifest pointing to different tags. diff --git a/internal/build/cli-compat.json b/internal/build/cli-compat.json new file mode 100644 index 0000000000..d3c37c23be --- /dev/null +++ b/internal/build/cli-compat.json @@ -0,0 +1,3 @@ +{ + "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 0000000000..22cb8f13a0 --- /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 0000000000..8d982db132 --- /dev/null +++ b/libs/clicompat/clicompat.go @@ -0,0 +1,450 @@ +package clicompat + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "slices" + "strings" + "sync" + "time" + + "github.com/databricks/cli/internal/build" + "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" + "golang.org/x/mod/semver" +) + +// ErrNotFound indicates that a requested resource (tag, branch, manifest) +// does not exist at the remote. +var ErrNotFound = errors.New("not found") + +// HTTPStatusError captures a non-200 HTTP status code from a manifest fetch. +type HTTPStatusError struct { + StatusCode int +} + +// Error implements the error interface. +func (e *HTTPStatusError) Error() string { + return fmt.Sprintf("HTTP %d fetching manifest", e.StatusCode) +} + +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 + + // 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 whose semver (0.0.0) is lower than + // all real CLI versions. These are treated as bleeding-edge and resolve to + // the highest versioned entry. + devVersionPrefix = "0.0.0-dev" + + maxFetchAttempts = 3 + 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. Not safe for parallel tests; the clicompat test +// suite does not use t.Parallel(). +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 +// +// Set DATABRICKS_FORCE_EMBEDDED_COMPAT=true to skip all tiers and use only +// the embedded manifest. Useful for local development when testing with a +// locally compiled binary. +// +// Set DATABRICKS_CACHE_ENABLED=false to bypass the local cache (tiers 1 and 3a), +// which is useful to recover from a bad cached manifest. +func FetchManifest(ctx context.Context) (Manifest, error) { + if force, ok := env.GetBool(ctx, "DATABRICKS_FORCE_EMBEDDED_COMPAT"); ok && force { + log.Debugf(ctx, "Using embedded manifest (DATABRICKS_FORCE_EMBEDDED_COMPAT=true)") + return parseEmbeddedManifest() + } + + cacheEnabled := true + if enabled, ok := env.GetBool(ctx, "DATABRICKS_CACHE_ENABLED"); ok { + cacheEnabled = enabled + } + + localPath := manifestLocalPath(ctx) + + // Read local file once — reuse across tiers. + var local cachedManifest + var localErr error + if cacheEnabled { + local, localErr = readLocalManifest(localPath) + } else { + localErr = errors.New("cache disabled") + } + + // 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 { + if cacheEnabled { + 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 := parseEmbeddedManifest() + 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) +} + +// parseEmbeddedManifest parses the embedded manifest once and caches the result. +var parseEmbeddedManifest = sync.OnceValues(func() (Manifest, error) { + return parseManifest(build.CLICompatManifestJSON) +}) + +// 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 := parseEmbeddedManifest() + 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 := parseEmbeddedManifest() + 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 + } + if errors.Is(err, ErrNotFound) { + return true + } + var httpErr *HTTPStatusError + if errors.As(err, &httpErr) && httpErr.StatusCode == http.StatusNotFound { + return true + } + // Git clone errors include "not found" in stderr when a branch/tag does not + // exist (e.g. "Remote branch X not found in upstream origin"). This is a + // pragmatic fallback until git.Clone wraps a typed error. + msg := strings.ToLower(err.Error()) + return strings.Contains(msg, "not found") +} + +// Resolve returns the manifest entry for the given CLI version. +// +// Each versioned entry defines a range floor: it applies to that CLI version +// and all versions above it, up to (but not including) the next entry. +// +// Resolution order: +// 1. Dev builds (version starts with "0.0.0-dev") use the highest versioned 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") + } + + // Collect and sort versioned keys descending. + versions := sortedVersions(m) + if len(versions) == 0 { + return Entry{}, errors.New("compatibility manifest has no versioned entries") + } + + // Dev builds (0.0.0-dev*) have semver lower than all real CLI versions, + // so they would incorrectly resolve to the lowest entry. Use the highest + // versioned entry instead, since dev builds represent the bleeding edge. + if strings.HasPrefix(cliVersion, devVersionPrefix) { + return m[versions[0]], nil + } + + // Exact match. + if entry, ok := m[cliVersion]; ok { + return entry, nil + } + + // 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. + return m[versions[len(versions)-1]], nil +} + +// sortedVersions returns manifest keys sorted descending by semver. +func sortedVersions(m Manifest) []string { + versions := make([]string, 0, len(m)) + for k := range m { + versions = append(versions, k) + } + slices.SortFunc(versions, func(a, b string) int { + return semver.Compare("v"+b, "v"+a) + }) + return versions +} + +// 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 { + log.Debugf(ctx, "Could not determine user cache directory: %v", err) + 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 for atomicity. +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.Warnf(ctx, "Failed to create cache directory %s: %v", dir, err) + return + } + tmp, err := os.CreateTemp(dir, ".compat-manifest-*.tmp") + if err != nil { + log.Warnf(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 + } + if err := os.Rename(tmpPath, path); err != nil { + log.Warnf(ctx, "Failed to rename temp cache file: %v", err) + } +} + +// --- Remote fetch --- + +// fetchRemoteWithRetry wraps fetchRemote with retries on transient errors. +// Client errors (4xx) are not retried since they will not recover. +func fetchRemoteWithRetry(ctx context.Context) (Manifest, error) { + var lastErr error + for attempt := range maxFetchAttempts { + if attempt > 0 { + log.Debugf(ctx, "Retrying manifest fetch (attempt %d/%d)", attempt+1, maxFetchAttempts) + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(fetchRetryBackoff): + } + } + m, err := fetchRemote(ctx) + if err == nil { + return m, nil + } + lastErr = err + + // Do not retry client errors (4xx) — they won't resolve on retry. + var httpErr *HTTPStatusError + if errors.As(err, &httpErr) && httpErr.StatusCode >= 400 && httpErr.StatusCode < 500 { + return nil, lastErr + } + } + 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 + } + req.Header.Set("User-Agent", "databricks-cli/"+build.GetInfo().Version) + + resp, err := httpClient.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, &HTTPStatusError{StatusCode: 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") + } + for k := range m { + if !semver.IsValid("v" + k) { + return nil, fmt.Errorf("invalid semver key %q in compatibility manifest", k) + } + } + for k, entry := range m { + if entry.AppKit == "" { + return nil, fmt.Errorf("manifest entry %q has empty appkit version", k) + } + if entry.AgentSkills == "" { + return nil, fmt.Errorf("manifest entry %q has empty skills version", k) + } + if !semver.IsValid("v" + entry.AppKit) { + return nil, fmt.Errorf("manifest entry %q has invalid appkit version %q", k, entry.AppKit) + } + if !semver.IsValid("v" + entry.AgentSkills) { + return nil, fmt.Errorf("manifest entry %q has invalid skills version %q", k, entry.AgentSkills) + } + } + return m, nil +} diff --git a/libs/clicompat/clicompat_test.go b/libs/clicompat/clicompat_test.go new file mode 100644 index 0000000000..87d29c6dfe --- /dev/null +++ b/libs/clicompat/clicompat_test.go @@ -0,0 +1,498 @@ +package clicompat + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io/fs" + "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()) +} + +// --- Resolve tests --- + +// TestResolve_Ranges verifies range-based resolution. Each versioned entry +// defines a range floor: it applies to that CLI version and all versions above +// it up to (but not including) the next entry. Dev builds use the highest +// versioned entry. The manifest uses distinct appkit values so assertions are +// unambiguous. +func TestResolve_Ranges(t *testing.T) { + m := Manifest{ + "0.296.0": {AppKit: "0.27.0", AgentSkills: "0.1.5"}, + "0.290.0": {AppKit: "0.24.0", AgentSkills: "0.1.4"}, + "0.280.0": {AppKit: "0.20.0", AgentSkills: "0.1.0"}, + } + + tests := []struct { + name string + cliVersion string + wantAppKit string + wantSkills string + }{ + {"exact match at range floor", "0.280.0", "0.20.0", "0.1.0"}, + {"mid-range", "0.285.0", "0.20.0", "0.1.0"}, + {"just below next range", "0.289.9", "0.20.0", "0.1.0"}, + {"exact match mid entry", "0.290.0", "0.24.0", "0.1.4"}, + {"between mid and top", "0.293.0", "0.24.0", "0.1.4"}, + {"exact match highest", "0.296.0", "0.27.0", "0.1.5"}, + {"newer than all entries uses highest", "0.300.0", "0.27.0", "0.1.5"}, + {"older than all entries uses lowest", "0.270.0", "0.20.0", "0.1.0"}, + {"dev build uses highest", "0.0.0-dev+abc123", "0.27.0", "0.1.5"}, + {"bare dev uses highest", "0.0.0-dev", "0.27.0", "0.1.5"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + entry, err := Resolve(m, tc.cliVersion) + require.NoError(t, err) + assert.Equal(t, tc.wantAppKit, entry.AppKit) + assert.Equal(t, tc.wantSkills, 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") +} + +// --- FetchManifest tests --- + +func TestFetchManifest_RemoteSuccess(t *testing.T) { + ctx := testContext(t) + want := Manifest{ + "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["0.296.0"].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["0.300.0"].AppKit, result["0.300.0"].AppKit) +} + +func TestFetchManifest_RemoteFailFallsBackToStaleCache(t *testing.T) { + ctx := testContext(t) + + // Pre-populate the local cache with a stale manifest. + staleManifest := Manifest{ + "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["0.296.0"].AppKit) +} + +func TestFetchManifest_RemoteSuccessWritesLocalCache(t *testing.T) { + ctx := testContext(t) + want := Manifest{ + "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{ + "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["0.296.0"].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["0.296.0"].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{ + "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["0.296.0"].AppKit) + assert.Equal(t, int32(2), callCount.Load(), "should have retried after first failure") +} + +// --- parseManifest tests --- + +func TestParseManifest_Valid(t *testing.T) { + data := `{"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["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_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{ + "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") + require.NotEmpty(t, m, "embedded manifest must have at least one entry") + + // All keys must be valid semver. + var keys []string + for k := range m { + assert.True(t, semver.IsValid("v"+k), "key %q must be valid semver", k) + keys = append(keys, k) + } + + // Sort to get deterministic order from map iteration. + slices.SortFunc(keys, func(a, b string) int { + return semver.Compare("v"+a, "v"+b) + }) + + // Keys must be in ascending semver order. + for i := 1; i < len(keys); i++ { + cmp := semver.Compare("v"+keys[i-1], "v"+keys[i]) + assert.LessOrEqual(t, cmp, 0, + "keys must be in ascending order: %s should come before %s", + keys[i-1], keys[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{ + "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["0.300.0"].AppKit) + assert.True(t, cached.isFresh(cacheTTL), "just-written file should be fresh") +} + +// --- IsNotFoundError tests --- + +func TestIsNotFoundError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + {name: "nil", err: nil, want: false}, + {name: "unrelated error", err: errors.New("connection refused"), want: false}, + {name: "sentinel ErrNotFound", err: ErrNotFound, want: true}, + {name: "wrapped ErrNotFound", err: fmt.Errorf("fetching: %w", ErrNotFound), want: true}, + {name: "HTTPStatusError 404", err: &HTTPStatusError{StatusCode: 404}, want: true}, + {name: "wrapped HTTPStatusError 404", err: fmt.Errorf("fetch failed: %w", &HTTPStatusError{StatusCode: 404}), want: true}, + {name: "HTTPStatusError 500", err: &HTTPStatusError{StatusCode: 500}, want: false}, + {name: "HTTPStatusError 403", err: &HTTPStatusError{StatusCode: 403}, want: false}, + {name: "git not found", err: errors.New("Remote branch template-v99 not found in upstream origin"), want: true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, IsNotFoundError(tc.err)) + }) + } +} + +// --- parseManifest entry validation tests --- + +func TestParseManifest_EmptyAppKit(t *testing.T) { + data := `{"0.296.0":{"appkit":"","skills":"0.1.5"}}` + _, err := parseManifest([]byte(data)) + require.Error(t, err) + assert.Contains(t, err.Error(), "empty appkit version") +} + +func TestParseManifest_EmptySkills(t *testing.T) { + data := `{"0.296.0":{"appkit":"0.27.0","skills":""}}` + _, err := parseManifest([]byte(data)) + require.Error(t, err) + assert.Contains(t, err.Error(), "empty skills version") +} + +func TestParseManifest_InvalidAppKitSemver(t *testing.T) { + data := `{"0.296.0":{"appkit":"not-semver","skills":"0.1.5"}}` + _, err := parseManifest([]byte(data)) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid appkit version") +} + +func TestParseManifest_InvalidSkillsSemver(t *testing.T) { + data := `{"0.296.0":{"appkit":"0.27.0","skills":"abc"}}` + _, err := parseManifest([]byte(data)) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid skills version") +} + +func TestParseManifest_InvalidKey(t *testing.T) { + data := `{"not-semver":{"appkit":"0.27.0","skills":"0.1.5"}}` + _, err := parseManifest([]byte(data)) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid semver key") +} + +// --- FetchManifest no-retry-on-404 test --- + +func TestFetchManifest_NoRetryOn404(t *testing.T) { + ctx := testContext(t) + var callCount atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount.Add(1) + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + redirectToServer(t, srv) + + // Should fall back to embedded manifest (since 404 is not retried). + result, err := FetchManifest(ctx) + require.NoError(t, err) + + embedded, _ := parseEmbeddedManifest() + assert.Equal(t, embedded["0.300.0"].AppKit, result["0.300.0"].AppKit) + assert.Equal(t, int32(1), callCount.Load(), "404 should not be retried") +} + +// --- FetchManifest cache-disabled test --- + +func TestFetchManifest_CacheDisabled(t *testing.T) { + ctx := testContext(t) + ctx = env.Set(ctx, "DATABRICKS_CACHE_ENABLED", "false") + + want := Manifest{ + "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 fetches from remote. + result1, err := FetchManifest(ctx) + require.NoError(t, err) + assert.Equal(t, "0.99.0", result1["0.296.0"].AppKit) + + // Second call should also fetch from remote (cache is disabled). + result2, err := FetchManifest(ctx) + require.NoError(t, err) + assert.Equal(t, "0.99.0", result2["0.296.0"].AppKit) + + assert.Equal(t, int32(2), callCount.Load(), "with cache disabled, both calls should hit the server") + + // Verify no cache file was written. + localPath := manifestLocalPath(ctx) + _, statErr := os.Stat(localPath) + assert.ErrorIs(t, statErr, fs.ErrNotExist, "cache file should not exist when cache is disabled") +} + +func TestFetchManifest_ForceEmbedded(t *testing.T) { + ctx := testContext(t) + ctx = env.Set(ctx, "DATABRICKS_FORCE_EMBEDDED_COMPAT", "true") + + var called bool + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + called = true + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + redirectToServer(t, srv) + + result, err := FetchManifest(ctx) + require.NoError(t, err) + assert.False(t, called, "server should not be called when DATABRICKS_FORCE_EMBEDDED_COMPAT=true") + + embedded, _ := parseManifest(build.CLICompatManifestJSON) + assert.Equal(t, embedded["0.300.0"].AppKit, result["0.300.0"].AppKit) +}