diff --git a/Taskfile.yml b/Taskfile.yml index 0d582a1954e..7a8fb12adef 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -615,6 +615,7 @@ tasks: - libs/aitools/** - experimental/aitools/** - acceptance/apps/** + - acceptance/experimental/aitools/** - "{{.EMBED_SOURCES}}" cmds: - | @@ -628,7 +629,7 @@ tasks: --format ${GOTESTSUM_FORMAT:-pkgname-and-test-fails} \ --no-summary=skipped \ --packages ./acceptance/... \ - -- -timeout=${LOCAL_TIMEOUT:-30m} -run "TestAccept/apps" + -- -timeout=${LOCAL_TIMEOUT:-30m} -run "TestAccept/(apps|experimental/aitools)" test-exp-ssh: desc: Run experimental SSH unit and acceptance tests diff --git a/acceptance/experimental/aitools/skills/install-experimental-empty/out.test.toml b/acceptance/experimental/aitools/skills/install-experimental-empty/out.test.toml new file mode 100644 index 00000000000..d6187dcb046 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-experimental-empty/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = [] diff --git a/acceptance/experimental/aitools/skills/install-experimental-empty/output.txt b/acceptance/experimental/aitools/skills/install-experimental-empty/output.txt new file mode 100644 index 00000000000..78a0d503215 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-experimental-empty/output.txt @@ -0,0 +1,9 @@ + +=== --experimental against a manifest with no experimental skills logs a nudge +>>> [CLI] experimental aitools install --global --experimental +Command "install" is deprecated, use "databricks aitools install" instead. +Flag --global has been deprecated, use --scope=global +Installing Databricks AI skills for Claude Code... +Using skills version test-ref +Warn: --experimental was set but the manifest at test-ref exposes no experimental skills. Set DATABRICKS_SKILLS_REF to a release that includes them (or =main for the latest). +Installed 1 skill. diff --git a/acceptance/experimental/aitools/skills/install-experimental-empty/script b/acceptance/experimental/aitools/skills/install-experimental-empty/script new file mode 100644 index 00000000000..cf27462115c --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-experimental-empty/script @@ -0,0 +1,5 @@ +# Agent detection needs ~/.claude; prefer USERPROFILE on Windows. +mkdir -p "${USERPROFILE:-$HOME}/.claude" + +title "--experimental against a manifest with no experimental skills logs a nudge" +trace $CLI experimental aitools install --global --experimental diff --git a/acceptance/experimental/aitools/skills/install-experimental-empty/test.toml b/acceptance/experimental/aitools/skills/install-experimental-empty/test.toml new file mode 100644 index 00000000000..0e77cc5247d --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-experimental-empty/test.toml @@ -0,0 +1,31 @@ +Env.DATABRICKS_SKILLS_BASE_URL = "$DATABRICKS_HOST" +Env.DATABRICKS_SKILLS_REF = "test-ref" + +Ignore = [ + ".databricks/aitools/skills/.state.json", +] + +[EnvMatrix] +DATABRICKS_BUNDLE_ENGINE = [] + +# Stable-only manifest (no repo_dir=experimental). +[[Server]] +Pattern = "GET /test-ref/manifest.json" +Response.Body = ''' +{ + "version": "2", + "updated_at": "2026-01-01T00:00:00Z", + "skills": { + "test-stable": {"version": "1.0.0", "files": ["SKILL.md"]} + } +} +''' + +[[Server]] +Pattern = "GET /test-ref/skills/test-stable/SKILL.md" +Response.Body = '''--- +name: test-stable +--- + +# Stable +''' diff --git a/acceptance/experimental/aitools/skills/install-specific/out.test.toml b/acceptance/experimental/aitools/skills/install-specific/out.test.toml new file mode 100644 index 00000000000..d6187dcb046 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-specific/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = [] diff --git a/acceptance/experimental/aitools/skills/install-specific/output.txt b/acceptance/experimental/aitools/skills/install-specific/output.txt new file mode 100644 index 00000000000..f6222311b1b --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-specific/output.txt @@ -0,0 +1,26 @@ + +=== install only one specific stable skill via --skills +>>> [CLI] experimental aitools install --global --skills test-stable-a +Command "install" is deprecated, use "databricks aitools install" instead. +Flag --global has been deprecated, use --scope=global +Installing Databricks AI skills for Claude Code... +Using skills version test-ref +Installed 1 skill. + +=== install a specific experimental skill +>>> [CLI] experimental aitools install --global --skills test-exp --experimental +Command "install" is deprecated, use "databricks aitools install" instead. +Flag --global has been deprecated, use --scope=global +Installing Databricks AI skills for Claude Code... +Using skills version test-ref +Installed 1 skill. + +=== asking for an experimental skill without --experimental flag errors out +>>> [CLI] experimental aitools install --global --skills test-exp +Command "install" is deprecated, use "databricks aitools install" instead. +Flag --global has been deprecated, use --scope=global +Installing Databricks AI skills for Claude Code... +Using skills version test-ref +Error: skill "test-exp" is experimental; use --experimental to install + +Exit code: 1 diff --git a/acceptance/experimental/aitools/skills/install-specific/script b/acceptance/experimental/aitools/skills/install-specific/script new file mode 100644 index 00000000000..f87aa4fbd24 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-specific/script @@ -0,0 +1,11 @@ +# Agent detection needs ~/.claude; prefer USERPROFILE on Windows. +mkdir -p "${USERPROFILE:-$HOME}/.claude" + +title "install only one specific stable skill via --skills" +trace $CLI experimental aitools install --global --skills test-stable-a + +title "install a specific experimental skill" +trace $CLI experimental aitools install --global --skills test-exp --experimental + +title "asking for an experimental skill without --experimental flag errors out" +errcode trace $CLI experimental aitools install --global --skills test-exp diff --git a/acceptance/experimental/aitools/skills/install-specific/test.toml b/acceptance/experimental/aitools/skills/install-specific/test.toml new file mode 100644 index 00000000000..9be89c8b92a --- /dev/null +++ b/acceptance/experimental/aitools/skills/install-specific/test.toml @@ -0,0 +1,50 @@ +Env.DATABRICKS_SKILLS_BASE_URL = "$DATABRICKS_HOST" +Env.DATABRICKS_SKILLS_REF = "test-ref" + +Ignore = [ + ".databricks/aitools/skills/.state.json", +] + +[EnvMatrix] +DATABRICKS_BUNDLE_ENGINE = [] + +[[Server]] +Pattern = "GET /test-ref/manifest.json" +Response.Body = ''' +{ + "version": "2", + "updated_at": "2026-01-01T00:00:00Z", + "skills": { + "test-stable-a": {"version": "1.0.0", "files": ["SKILL.md"], "repo_dir": "skills"}, + "test-stable-b": {"version": "1.0.0", "files": ["SKILL.md"], "repo_dir": "skills"}, + "test-exp": {"version": "0.0.1", "files": ["SKILL.md"], "repo_dir": "experimental"} + } +} +''' + +[[Server]] +Pattern = "GET /test-ref/skills/test-stable-a/SKILL.md" +Response.Body = '''--- +name: test-stable-a +--- + +# A +''' + +[[Server]] +Pattern = "GET /test-ref/skills/test-stable-b/SKILL.md" +Response.Body = '''--- +name: test-stable-b +--- + +# B +''' + +[[Server]] +Pattern = "GET /test-ref/experimental/test-exp/SKILL.md" +Response.Body = '''--- +name: test-exp +--- + +# Exp +''' diff --git a/acceptance/experimental/aitools/skills/install/out.test.toml b/acceptance/experimental/aitools/skills/install/out.test.toml new file mode 100644 index 00000000000..d6187dcb046 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = [] diff --git a/acceptance/experimental/aitools/skills/install/output.txt b/acceptance/experimental/aitools/skills/install/output.txt new file mode 100644 index 00000000000..f659c9e2998 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install/output.txt @@ -0,0 +1,24 @@ + +=== stable-only install (no --experimental flag) installs 1 skill +>>> [CLI] experimental aitools install --global +Command "install" is deprecated, use "databricks aitools install" instead. +Flag --global has been deprecated, use --scope=global +Installing Databricks AI skills for Claude Code... +Using skills version test-ref +Installed 1 skill. + +=== re-run with --experimental installs the experimental one too +>>> [CLI] experimental aitools install --global --experimental +Command "install" is deprecated, use "databricks aitools install" instead. +Flag --global has been deprecated, use --scope=global +Installing Databricks AI skills for Claude Code... +Using skills version test-ref +Installed 2 skills. + +=== no-op re-run is idempotent (no new fetches, no errors) +>>> [CLI] experimental aitools install --global --experimental +Command "install" is deprecated, use "databricks aitools install" instead. +Flag --global has been deprecated, use --scope=global +Installing Databricks AI skills for Claude Code... +Using skills version test-ref +Installed 2 skills. diff --git a/acceptance/experimental/aitools/skills/install/script b/acceptance/experimental/aitools/skills/install/script new file mode 100644 index 00000000000..cc64fb54951 --- /dev/null +++ b/acceptance/experimental/aitools/skills/install/script @@ -0,0 +1,11 @@ +# Agent detection needs ~/.claude; prefer USERPROFILE on Windows. +mkdir -p "${USERPROFILE:-$HOME}/.claude" + +title "stable-only install (no --experimental flag) installs 1 skill" +trace $CLI experimental aitools install --global + +title "re-run with --experimental installs the experimental one too" +trace $CLI experimental aitools install --global --experimental + +title "no-op re-run is idempotent (no new fetches, no errors)" +trace $CLI experimental aitools install --global --experimental diff --git a/acceptance/experimental/aitools/skills/install/test.toml b/acceptance/experimental/aitools/skills/install/test.toml new file mode 100644 index 00000000000..f21efd9189a --- /dev/null +++ b/acceptance/experimental/aitools/skills/install/test.toml @@ -0,0 +1,52 @@ +# Mock server replaces raw.githubusercontent.com for manifest + skill files. +Env.DATABRICKS_SKILLS_BASE_URL = "$DATABRICKS_HOST" +Env.DATABRICKS_SKILLS_REF = "test-ref" + +Ignore = [ + ".databricks/aitools/skills/.state.json", +] + +[EnvMatrix] +DATABRICKS_BUNDLE_ENGINE = [] + +# Stable + experimental skills via repo_dir. +[[Server]] +Pattern = "GET /test-ref/manifest.json" +Response.Body = ''' +{ + "version": "2", + "updated_at": "2026-01-01T00:00:00Z", + "skills": { + "test-stable": { + "version": "1.0.0", + "description": "Stable test skill", + "files": ["SKILL.md"], + "repo_dir": "skills" + }, + "test-exp": { + "version": "0.0.1", + "description": "Experimental test skill", + "files": ["SKILL.md"], + "repo_dir": "experimental" + } + } +} +''' + +[[Server]] +Pattern = "GET /test-ref/skills/test-stable/SKILL.md" +Response.Body = '''--- +name: test-stable +--- + +# Test stable skill +''' + +[[Server]] +Pattern = "GET /test-ref/experimental/test-exp/SKILL.md" +Response.Body = '''--- +name: test-exp +--- + +# Test experimental skill +''' diff --git a/cmd/aitools/list.go b/cmd/aitools/list.go index 62d17f3664c..22057c007bd 100644 --- a/cmd/aitools/list.go +++ b/cmd/aitools/list.go @@ -115,7 +115,7 @@ func defaultListSkills(cmd *cobra.Command, scope string) error { meta := manifest.Skills[name] tag := "" - if meta.Experimental { + if meta.IsExperimental() { tag = " [experimental]" } diff --git a/libs/aitools/installer/installer.go b/libs/aitools/installer/installer.go index b905202d4c5..548be8dcf3e 100644 --- a/libs/aitools/installer/installer.go +++ b/libs/aitools/installer/installer.go @@ -24,14 +24,33 @@ import ( ) const ( - skillsRepoOwner = "databricks" - skillsRepoName = "databricks-agent-skills" - skillsRepoPath = "skills" + skillsRepoOwner = "databricks" + skillsRepoName = "databricks-agent-skills" + stableSkillsRepoPath = "skills" + experimentalRepoPath = "experimental" ) +func manifestHasExperimental(m *Manifest) bool { + for _, meta := range m.Skills { + if meta.IsExperimental() { + return true + } + } + return false +} + +func stateRepoDir(state *InstallState, name string) string { + if state != nil && state.RepoDirs != nil { + if repoDir := state.RepoDirs[name]; repoDir != "" { + return repoDir + } + } + return stableSkillsRepoPath +} + // fetchFileFn is the function used to download individual skill files. // It is a package-level var so tests can replace it with a mock. -var fetchFileFn = fetchSkillFile +var fetchFileFn func(ctx context.Context, ref, repoDir, skillName, filePath string) ([]byte, error) = fetchSkillFile // 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). @@ -47,6 +66,15 @@ func GetSkillsRef(ctx context.Context) (ref string, explicit bool, err error) { return "v" + v, false, nil } +// GetSkillsBaseURL returns the manifest and skill fetch base URL. +// DATABRICKS_SKILLS_BASE_URL overrides GitHub raw URLs for acceptance tests. +func GetSkillsBaseURL(ctx context.Context) string { + if base := env.Get(ctx, "DATABRICKS_SKILLS_BASE_URL"); base != "" { + return strings.TrimRight(base, "/") + } + return "https://raw.githubusercontent.com/" + skillsRepoOwner + "/" + skillsRepoName +} + // Manifest describes the skills manifest fetched from the skills repo. type Manifest struct { Version string `json:"version"` @@ -56,12 +84,22 @@ type Manifest struct { // SkillMeta describes a single skill entry in the manifest. type SkillMeta struct { - Version string `json:"version"` - UpdatedAt string `json:"updated_at"` - Files []string `json:"files"` - Experimental bool `json:"experimental,omitempty"` - Description string `json:"description,omitempty"` - MinCLIVer string `json:"min_cli_version,omitempty"` + Version string `json:"version"` + UpdatedAt string `json:"updated_at"` + Files []string `json:"files"` + Description string `json:"description,omitempty"` + MinCLIVer string `json:"min_cli_version,omitempty"` + + // RepoDir is "skills" or "experimental" (manifest field repo_dir). + RepoDir string `json:"repo_dir,omitempty"` + + // SourceName is the upstream skill directory name within RepoDir. + // Set during normalization, not from JSON. + SourceName string `json:"-"` +} + +func (s SkillMeta) IsExperimental() bool { + return s.RepoDir == experimentalRepoPath } // InstallOptions controls the behavior of InstallSkillsForAgents. @@ -71,9 +109,12 @@ type InstallOptions struct { Scope string // ScopeGlobal or ScopeProject (default: global) } -func fetchSkillFile(ctx context.Context, ref, skillName, filePath string) ([]byte, error) { - url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/%s/%s/%s", - skillsRepoOwner, skillsRepoName, ref, skillsRepoPath, skillName, filePath) +func fetchSkillFile(ctx context.Context, ref, repoDir, skillName, filePath string) ([]byte, error) { + if repoDir == "" { + repoDir = stableSkillsRepoPath + } + url := fmt.Sprintf("%s/%s/%s/%s/%s", + GetSkillsBaseURL(ctx), ref, repoDir, skillName, filePath) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { @@ -128,6 +169,10 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent return err } + if opts.IncludeExperimental && !manifestHasExperimental(manifest) { + log.Warnf(ctx, "--experimental was set but the manifest at %s exposes no experimental skills. Set DATABRICKS_SKILLS_REF to a release that includes them (or =main for the latest).", ref) + } + scope := opts.Scope if scope == "" { scope = ScopeGlobal @@ -186,9 +231,10 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent for _, name := range skillNames { meta := targetSkills[name] + // Idempotency: skip if same version is already installed, the canonical // dir exists, AND every requested agent already has the skill on disk. - if state != nil && state.Skills[name] == meta.Version { + if state != nil && state.Skills[name] == meta.Version && stateRepoDir(state, name) == meta.RepoDir { skillDir := filepath.Join(baseDir, name) if _, statErr := os.Stat(skillDir); statErr == nil && allAgentsHaveSkill(ctx, name, targetAgents, scope, cwd) { log.Debugf(ctx, "%s v%s already installed for all agents, skipping", name, meta.Version) @@ -196,7 +242,7 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent } } - if err := installSkillForAgents(ctx, name, meta.Files, targetAgents, params); err != nil { + if err := installSkillForAgents(ctx, name, meta, targetAgents, params); err != nil { return err } } @@ -207,8 +253,12 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent state = &InstallState{ SchemaVersion: 1, Skills: make(map[string]string, len(targetSkills)), + RepoDirs: make(map[string]string, len(targetSkills)), } } + if state.RepoDirs == nil { + state.RepoDirs = make(map[string]string, len(state.Skills)+len(targetSkills)) + } state.Release = ref state.LastUpdated = time.Now() // IncludeExperimental reflects the last invocation's flag value. The Skills @@ -218,6 +268,7 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent state.Scope = scope for name, meta := range targetSkills { state.Skills[name] = meta.Version + state.RepoDirs[name] = meta.RepoDir } if err := SaveState(baseDir, state); err != nil { return err @@ -287,7 +338,7 @@ func resolveSkills(ctx context.Context, skills map[string]SkillMeta, opts Instal result := make(map[string]SkillMeta, len(candidates)) for name, meta := range candidates { - if meta.Experimental && !opts.IncludeExperimental { + if meta.IsExperimental() && !opts.IncludeExperimental { if isSpecific { return nil, fmt.Errorf("skill %q is experimental; use --experimental to install", name) } @@ -394,9 +445,9 @@ type installParams struct { ref string } -func installSkillForAgents(ctx context.Context, skillName string, files []string, detectedAgents []*agents.Agent, params installParams) error { +func installSkillForAgents(ctx context.Context, skillName string, meta SkillMeta, detectedAgents []*agents.Agent, params installParams) error { canonicalDir := filepath.Join(params.baseDir, skillName) - if err := installSkillToDir(ctx, params.ref, skillName, canonicalDir, files); err != nil { + if err := installSkillToDir(ctx, params.ref, meta.RepoDir, meta.SourceName, canonicalDir, meta.Files); err != nil { return err } @@ -494,7 +545,7 @@ func backupThirdPartySkill(ctx context.Context, destDir, canonicalDir, skillName return nil } -func installSkillToDir(ctx context.Context, ref, skillName, destDir string, files []string) error { +func installSkillToDir(ctx context.Context, ref, repoDir, skillName, destDir string, files []string) error { // remove existing skill directory for clean install if err := os.RemoveAll(destDir); err != nil { return fmt.Errorf("failed to remove existing skill: %w", err) @@ -505,7 +556,7 @@ func installSkillToDir(ctx context.Context, ref, skillName, destDir string, file } for _, file := range files { - content, err := fetchFileFn(ctx, ref, skillName, file) + content, err := fetchFileFn(ctx, ref, repoDir, skillName, file) if err != nil { return err } diff --git a/libs/aitools/installer/installer_test.go b/libs/aitools/installer/installer_test.go index 3ace56e952f..710f4861439 100644 --- a/libs/aitools/installer/installer_test.go +++ b/libs/aitools/installer/installer_test.go @@ -29,6 +29,7 @@ func (m *mockManifestSource) FetchManifest(_ context.Context, _ string) (*Manife if m.fetchErr != nil { return nil, m.fetchErr } + normalizeManifest(m.manifest) return m.manifest, nil } @@ -53,7 +54,7 @@ func setupFetchMock(t *testing.T) { t.Helper() orig := fetchFileFn t.Cleanup(func() { fetchFileFn = orig }) - fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) { + fetchFileFn = func(_ context.Context, _, _, skillName, filePath string) ([]byte, error) { return []byte("# " + skillName + "/" + filePath), nil } } @@ -261,10 +262,10 @@ func TestExperimentalSkillsSkippedByDefault(t *testing.T) { t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) manifest := testManifest() - manifest.Skills["databricks-experimental"] = SkillMeta{ - Version: "0.1.0", - Files: []string{"SKILL.md"}, - Experimental: true, + manifest.Skills["databricks-iceberg"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + RepoDir: experimentalRepoPath, } src := &mockManifestSource{manifest: manifest} @@ -278,7 +279,7 @@ func TestExperimentalSkillsSkippedByDefault(t *testing.T) { require.NoError(t, err) // Only non-experimental skills should be installed. assert.Len(t, state.Skills, 2) - assert.NotContains(t, state.Skills, "databricks-experimental") + assert.NotContains(t, state.Skills, "databricks-iceberg") assert.Contains(t, stderr.String(), "Installed 2 skills.") } @@ -290,10 +291,10 @@ func TestExperimentalSkillsIncludedWithFlag(t *testing.T) { t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) manifest := testManifest() - manifest.Skills["databricks-experimental"] = SkillMeta{ - Version: "0.1.0", - Files: []string{"SKILL.md"}, - Experimental: true, + manifest.Skills["databricks-iceberg"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + RepoDir: experimentalRepoPath, } src := &mockManifestSource{manifest: manifest} @@ -308,7 +309,7 @@ func TestExperimentalSkillsIncludedWithFlag(t *testing.T) { state, err := LoadState(globalDir) require.NoError(t, err) assert.Len(t, state.Skills, 3) - assert.Contains(t, state.Skills, "databricks-experimental") + assert.Contains(t, state.Skills, "databricks-iceberg") assert.True(t, state.IncludeExperimental) assert.Contains(t, stderr.String(), "Installed 3 skills.") @@ -392,7 +393,7 @@ func TestIdempotentSecondInstallSkips(t *testing.T) { fetchCalls := 0 orig := fetchFileFn t.Cleanup(func() { fetchFileFn = orig }) - fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) { + fetchFileFn = func(_ context.Context, _, _, skillName, filePath string) ([]byte, error) { fetchCalls++ return []byte("# " + skillName + "/" + filePath), nil } @@ -430,7 +431,7 @@ func TestIdempotentInstallUpdatesNewVersions(t *testing.T) { var fetchedSkills []string orig := fetchFileFn t.Cleanup(func() { fetchFileFn = orig }) - fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) { + fetchFileFn = func(_ context.Context, _, _, skillName, filePath string) ([]byte, error) { fetchedSkills = append(fetchedSkills, skillName) return []byte("# " + skillName + "/" + filePath), nil } @@ -517,7 +518,7 @@ func TestIdempotentInstallReinstallsForNewAgent(t *testing.T) { fetchCalls := 0 orig := fetchFileFn t.Cleanup(func() { fetchFileFn = orig }) - fetchFileFn = func(_ context.Context, _, skillName, filePath string) ([]byte, error) { + fetchFileFn = func(_ context.Context, _, _, skillName, filePath string) ([]byte, error) { fetchCalls++ return []byte("# " + skillName + "/" + filePath), nil } @@ -727,6 +728,53 @@ func TestInstallProjectScopeZeroCompatibleAgentsReturnsError(t *testing.T) { assert.Contains(t, err.Error(), "No Project Agent") } +func TestInstallKeepsNameWhenRepoDirChanges(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + agent := testAgent(tmp) + var fetchedFrom []string + orig := fetchFileFn + t.Cleanup(func() { fetchFileFn = orig }) + fetchFileFn = func(_ context.Context, _, repoDir, skillName, filePath string) ([]byte, error) { + fetchedFrom = append(fetchedFrom, filepath.Join(repoDir, skillName, filePath)) + return []byte("# " + skillName + "/" + filePath), nil + } + + stableManifest := &Manifest{ + Version: "1", + Skills: map[string]SkillMeta{ + "databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}}, + }, + } + require.NoError(t, InstallSkillsForAgents( + ctx, &mockManifestSource{manifest: stableManifest}, + []*agents.Agent{agent}, InstallOptions{}, + )) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + require.DirExists(t, filepath.Join(globalDir, "databricks-jobs")) + assert.Contains(t, fetchedFrom, filepath.Join(stableSkillsRepoPath, "databricks-jobs", "SKILL.md")) + fetchedFrom = nil + + experimentalManifest := &Manifest{ + Version: "1", + Skills: map[string]SkillMeta{ + "databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}, RepoDir: experimentalRepoPath}, + }, + } + require.NoError(t, InstallSkillsForAgents( + ctx, &mockManifestSource{manifest: experimentalManifest}, + []*agents.Agent{agent}, InstallOptions{IncludeExperimental: true}, + )) + + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.Equal(t, "0.1.0", state.Skills["databricks-jobs"]) + assert.Equal(t, experimentalRepoPath, state.RepoDirs["databricks-jobs"]) + assert.DirExists(t, filepath.Join(globalDir, "databricks-jobs")) + assert.Contains(t, fetchedFrom, filepath.Join(experimentalRepoPath, "databricks-jobs", "SKILL.md")) +} + func TestSupportsProjectScopeSetCorrectly(t *testing.T) { expected := map[string]bool{ "claude-code": true, diff --git a/libs/aitools/installer/source.go b/libs/aitools/installer/source.go index 5dc78a254fd..f3d9842f876 100644 --- a/libs/aitools/installer/source.go +++ b/libs/aitools/installer/source.go @@ -23,8 +23,7 @@ type GitHubManifestSource struct{} // FetchManifest fetches the skills manifest from GitHub at the given ref. func (s *GitHubManifestSource) FetchManifest(ctx context.Context, ref string) (*Manifest, error) { log.Debugf(ctx, "Fetching skills manifest from %s/%s@%s", skillsRepoOwner, skillsRepoName, ref) - url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/manifest.json", - skillsRepoOwner, skillsRepoName, ref) + url := fmt.Sprintf("%s/%s/manifest.json", GetSkillsBaseURL(ctx), ref) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { @@ -50,5 +49,20 @@ func (s *GitHubManifestSource) FetchManifest(ctx context.Context, ref string) (* return nil, fmt.Errorf("failed to parse manifest: %w", err) } + normalizeManifest(&manifest) return &manifest, nil } + +// normalizeManifest stamps SourceName and defaults RepoDir for older manifests. +func normalizeManifest(m *Manifest) { + if m.Skills == nil { + m.Skills = map[string]SkillMeta{} + } + for name, meta := range m.Skills { + if meta.RepoDir == "" { + meta.RepoDir = stableSkillsRepoPath + } + meta.SourceName = name + m.Skills[name] = meta + } +} diff --git a/libs/aitools/installer/source_test.go b/libs/aitools/installer/source_test.go new file mode 100644 index 00000000000..fb52563b686 --- /dev/null +++ b/libs/aitools/installer/source_test.go @@ -0,0 +1,62 @@ +package installer + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNormalizeManifestStampsSourceNameAndRepoDir(t *testing.T) { + m := &Manifest{ + Skills: map[string]SkillMeta{ + "databricks-apps": {Version: "0.1.0", Files: []string{"SKILL.md"}}, + "databricks-iceberg": {Version: "0.0.1", Files: []string{"SKILL.md"}, RepoDir: experimentalRepoPath}, + }, + } + + normalizeManifest(m) + + stable := m.Skills["databricks-apps"] + assert.False(t, stable.IsExperimental()) + assert.Equal(t, stableSkillsRepoPath, stable.RepoDir) + assert.Equal(t, "databricks-apps", stable.SourceName) + + exp, ok := m.Skills["databricks-iceberg"] + assert.True(t, ok) + assert.True(t, exp.IsExperimental()) + assert.Equal(t, experimentalRepoPath, exp.RepoDir) + assert.Equal(t, "databricks-iceberg", exp.SourceName) +} + +func TestManifestHasExperimental(t *testing.T) { + stableOnly := &Manifest{Skills: map[string]SkillMeta{ + "databricks-apps": {Version: "0.1.0"}, + }} + normalizeManifest(stableOnly) + assert.False(t, manifestHasExperimental(stableOnly)) + + withExperimental := &Manifest{ + Skills: map[string]SkillMeta{ + "databricks-apps": {Version: "0.1.0"}, + "databricks-iceberg": {Version: "0.0.1", RepoDir: experimentalRepoPath}, + }, + } + normalizeManifest(withExperimental) + assert.True(t, manifestHasExperimental(withExperimental)) +} + +func TestNormalizeManifestOnlyExperimentalSkills(t *testing.T) { + m := &Manifest{ + Skills: map[string]SkillMeta{ + "x": {Version: "0.0.1", RepoDir: experimentalRepoPath}, + }, + } + + normalizeManifest(m) + + got, ok := m.Skills["x"] + assert.True(t, ok) + assert.True(t, got.IsExperimental()) + assert.Equal(t, experimentalRepoPath, got.RepoDir) + assert.Equal(t, "x", got.SourceName) +} diff --git a/libs/aitools/installer/state.go b/libs/aitools/installer/state.go index a666a585057..9aa52359bd5 100644 --- a/libs/aitools/installer/state.go +++ b/libs/aitools/installer/state.go @@ -27,6 +27,7 @@ type InstallState struct { Release string `json:"release"` LastUpdated time.Time `json:"last_updated"` Skills map[string]string `json:"skills"` + RepoDirs map[string]string `json:"repo_dirs,omitempty"` Scope string `json:"scope,omitempty"` } diff --git a/libs/aitools/installer/state_test.go b/libs/aitools/installer/state_test.go index f1fcdb8c229..4ed1e78d5f2 100644 --- a/libs/aitools/installer/state_test.go +++ b/libs/aitools/installer/state_test.go @@ -26,6 +26,9 @@ func TestSaveAndLoadStateRoundtrip(t *testing.T) { Skills: map[string]string{ "databricks": "1.0.0", }, + RepoDirs: map[string]string{ + "databricks": stableSkillsRepoPath, + }, } err := SaveState(dir, original) @@ -105,6 +108,10 @@ func TestSaveAndLoadStateWithOptionalFields(t *testing.T) { "databricks": "1.0.0", "sql-tools": "0.2.0", }, + RepoDirs: map[string]string{ + "databricks": stableSkillsRepoPath, + "sql-tools": experimentalRepoPath, + }, Scope: "project", } diff --git a/libs/aitools/installer/uninstall.go b/libs/aitools/installer/uninstall.go index da4cc6378c0..2e97fa784a5 100644 --- a/libs/aitools/installer/uninstall.go +++ b/libs/aitools/installer/uninstall.go @@ -64,13 +64,13 @@ func UninstallSkillsOpts(ctx context.Context, opts UninstallOptions) error { if len(opts.Skills) > 0 { seen := make(map[string]bool) for _, name := range opts.Skills { + if _, ok := state.Skills[name]; !ok { + return fmt.Errorf("skill %q is not installed", name) + } if seen[name] { continue } seen[name] = true - if _, ok := state.Skills[name]; !ok { - return fmt.Errorf("skill %q is not installed", name) - } toRemove = append(toRemove, name) } } else { @@ -89,6 +89,7 @@ func UninstallSkillsOpts(ctx context.Context, opts UninstallOptions) error { log.Warnf(ctx, "Failed to remove %s: %v", canonicalDir, err) } delete(state.Skills, name) + delete(state.RepoDirs, name) } if removeAll { diff --git a/libs/aitools/installer/update.go b/libs/aitools/installer/update.go index 9965ff7fc63..39db94c8f62 100644 --- a/libs/aitools/installer/update.go +++ b/libs/aitools/installer/update.go @@ -113,19 +113,17 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent for _, name := range names { meta, inManifest := manifest.Skills[name] - oldVersion := state.Skills[name] if !inManifest { - _, wasInstalled := state.Skills[name] - if wasInstalled { + if _, ok := state.Skills[name]; ok { log.Warnf(ctx, "Warning: %q not found in manifest %s (keeping installed version).", name, latestTag) + result.Unchanged = append(result.Unchanged, name) } - result.Unchanged = append(result.Unchanged, name) continue } // Filter experimental skills unless state opted in. - if meta.Experimental && !state.IncludeExperimental { + if meta.IsExperimental() && !state.IncludeExperimental { log.Debugf(ctx, "Skipping experimental skill %s", name) result.Skipped = append(result.Skipped, name) continue @@ -138,10 +136,9 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent continue } - // Check if this is a new skill (not in state). - _, wasInstalled := state.Skills[name] + oldVersion, wasInstalled := state.Skills[name] - if meta.Version == oldVersion && !opts.Force { + if meta.Version == oldVersion && stateRepoDir(state, name) == meta.RepoDir && !opts.Force { result.Unchanged = append(result.Unchanged, name) continue } @@ -152,10 +149,10 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent NewVersion: meta.Version, } - if !wasInstalled { - result.Added = append(result.Added, update) - } else { + if wasInstalled { result.Updated = append(result.Updated, update) + } else { + result.Added = append(result.Added, update) } } @@ -177,7 +174,7 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent for _, change := range allChanges { meta := manifest.Skills[change.Name] - if err := installSkillForAgents(ctx, change.Name, meta.Files, targetAgents, params); err != nil { + if err := installSkillForAgents(ctx, change.Name, meta, targetAgents, params); err != nil { return nil, err } } @@ -185,8 +182,13 @@ func UpdateSkills(ctx context.Context, src ManifestSource, targetAgents []*agent // Update state. state.Release = latestTag state.LastUpdated = time.Now() + if state.RepoDirs == nil { + state.RepoDirs = make(map[string]string, len(state.Skills)+len(allChanges)) + } for _, change := range allChanges { + meta := manifest.Skills[change.Name] state.Skills[change.Name] = change.NewVersion + state.RepoDirs[change.Name] = meta.RepoDir } if err := SaveState(baseDir, state); err != nil { return nil, err @@ -200,7 +202,6 @@ func buildUpdateSkillSet(state *InstallState, manifest *Manifest, opts UpdateOpt skillSet := make(map[string]bool) if len(opts.Skills) > 0 { - // Only named skills. for _, name := range opts.Skills { skillSet[name] = true } diff --git a/libs/aitools/installer/update_test.go b/libs/aitools/installer/update_test.go index a82be1d0289..72f5a6b25f2 100644 --- a/libs/aitools/installer/update_test.go +++ b/libs/aitools/installer/update_test.go @@ -132,7 +132,7 @@ func TestUpdateCheckDryRun(t *testing.T) { fetchCalls := 0 orig := fetchFileFn t.Cleanup(func() { fetchFileFn = orig }) - fetchFileFn = func(_ context.Context, _, _, _ string) ([]byte, error) { + fetchFileFn = func(_ context.Context, _, _, _, _ string) ([]byte, error) { fetchCalls++ return []byte("content"), nil } @@ -169,7 +169,7 @@ func TestUpdateForceRedownloads(t *testing.T) { fetchCalls := 0 orig := fetchFileFn t.Cleanup(func() { fetchFileFn = orig }) - fetchFileFn = func(_ context.Context, _, _, _ string) ([]byte, error) { + fetchFileFn = func(_ context.Context, _, _, _, _ string) ([]byte, error) { fetchCalls++ return []byte("content"), nil } @@ -345,10 +345,10 @@ func TestUpdateSkipsExperimentalSkills(t *testing.T) { // New manifest with an experimental skill. updatedManifest := testManifest() - updatedManifest.Skills["databricks-experimental"] = SkillMeta{ - Version: "0.1.0", - Files: []string{"SKILL.md"}, - Experimental: true, + updatedManifest.Skills["databricks-iceberg"] = SkillMeta{ + Version: "0.1.0", + Files: []string{"SKILL.md"}, + RepoDir: experimentalRepoPath, } src2 := &mockManifestSource{manifest: updatedManifest} @@ -356,10 +356,162 @@ func TestUpdateSkipsExperimentalSkills(t *testing.T) { require.NoError(t, err) // Experimental skill should be skipped. - assert.Contains(t, result.Skipped, "databricks-experimental") + assert.Contains(t, result.Skipped, "databricks-iceberg") assert.Empty(t, result.Added) } +func TestUpdateKeepsNameWhenRepoDirChanges(t *testing.T) { + tests := []struct { + name string + installedManifest func() *Manifest + updatedManifest func() *Manifest + wantRepoDir string + }{ + { + name: "stable to experimental", + installedManifest: func() *Manifest { + return &Manifest{ + Version: "1", + Skills: map[string]SkillMeta{ + "databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}}, + }, + } + }, + updatedManifest: func() *Manifest { + return &Manifest{ + Version: "1", + Skills: map[string]SkillMeta{ + "databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}, RepoDir: experimentalRepoPath}, + }, + } + }, + wantRepoDir: experimentalRepoPath, + }, + { + name: "experimental to stable", + installedManifest: func() *Manifest { + return &Manifest{ + Version: "1", + Skills: map[string]SkillMeta{ + "databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}, RepoDir: experimentalRepoPath}, + }, + } + }, + updatedManifest: func() *Manifest { + return &Manifest{ + Version: "1", + Skills: map[string]SkillMeta{ + "databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}}, + }, + } + }, + wantRepoDir: stableSkillsRepoPath, + }, + } + + for _, tt := range tests { + for _, targeted := range []bool{false, true} { + mode := "all" + opts := UpdateOptions{} + if targeted { + mode = "targeted" + opts.Skills = []string{"databricks-jobs"} + } + + t.Run(tt.name+" "+mode, func(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + setupFetchMock(t) + t.Setenv("DATABRICKS_SKILLS_REF", testSkillsRef) + agent := testAgent(tmp) + + require.NoError(t, InstallSkillsForAgents( + ctx, &mockManifestSource{manifest: tt.installedManifest()}, + []*agents.Agent{agent}, InstallOptions{IncludeExperimental: true}, + )) + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + require.DirExists(t, filepath.Join(globalDir, "databricks-jobs")) + + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.0") + result, err := UpdateSkills( + ctx, &mockManifestSource{manifest: tt.updatedManifest()}, + []*agents.Agent{agent}, opts, + ) + require.NoError(t, err) + + require.Len(t, result.Updated, 1) + assert.Equal(t, "databricks-jobs", result.Updated[0].Name) + assert.Equal(t, "0.1.0", result.Updated[0].OldVersion) + assert.Equal(t, "0.1.0", result.Updated[0].NewVersion) + assert.NotContains(t, result.Unchanged, "databricks-jobs") + assert.Empty(t, result.Added) + + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.Equal(t, "0.1.0", state.Skills["databricks-jobs"]) + assert.Equal(t, tt.wantRepoDir, state.RepoDirs["databricks-jobs"]) + assert.DirExists(t, filepath.Join(globalDir, "databricks-jobs")) + }) + } + } +} + +func TestUpdateRepoDirChangeFromLegacyState(t *testing.T) { + tmp := setupTestHome(t) + ctx := cmdio.MockDiscard(t.Context()) + agent := testAgent(tmp) + t.Setenv("DATABRICKS_SKILLS_REF", "v0.2.0") + + globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills") + require.NoError(t, SaveState(globalDir, &InstallState{ + SchemaVersion: 1, + IncludeExperimental: true, + Release: testSkillsRef, + Skills: map[string]string{ + "databricks-jobs": "0.1.0", + }, + })) + + fetchCalls := 0 + orig := fetchFileFn + t.Cleanup(func() { fetchFileFn = orig }) + fetchFileFn = func(_ context.Context, _, _, _, _ string) ([]byte, error) { + fetchCalls++ + return []byte("content"), nil + } + + stableManifest := &Manifest{ + Version: "1", + Skills: map[string]SkillMeta{ + "databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}}, + }, + } + stableResult, err := UpdateSkills(ctx, &mockManifestSource{manifest: stableManifest}, []*agents.Agent{agent}, UpdateOptions{}) + require.NoError(t, err) + assert.Contains(t, stableResult.Unchanged, "databricks-jobs") + assert.Equal(t, 0, fetchCalls) + + t.Setenv("DATABRICKS_SKILLS_REF", "v0.3.0") + experimentalManifest := &Manifest{ + Version: "1", + Skills: map[string]SkillMeta{ + "databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}, RepoDir: experimentalRepoPath}, + }, + } + experimentalResult, err := UpdateSkills(ctx, &mockManifestSource{manifest: experimentalManifest}, []*agents.Agent{agent}, UpdateOptions{}) + require.NoError(t, err) + require.Len(t, experimentalResult.Updated, 1) + assert.Equal(t, "databricks-jobs", experimentalResult.Updated[0].Name) + assert.Equal(t, "0.1.0", experimentalResult.Updated[0].OldVersion) + assert.Equal(t, "0.1.0", experimentalResult.Updated[0].NewVersion) + assert.Positive(t, fetchCalls) + + state, err := LoadState(globalDir) + require.NoError(t, err) + assert.Equal(t, experimentalRepoPath, state.RepoDirs["databricks-jobs"]) +} + func TestUpdateSkipsMinCLIVersionSkills(t *testing.T) { tmp := setupTestHome(t) ctx := cmdio.MockDiscard(t.Context())