diff --git a/experimental/aitools/lib/agents/skills.go b/experimental/aitools/lib/agents/skills.go index 3c0d67dfbb..0ba522e356 100644 --- a/experimental/aitools/lib/agents/skills.go +++ b/experimental/aitools/lib/agents/skills.go @@ -14,12 +14,15 @@ const ( databricksSkillPrefix = "databricks" // CanonicalSkillsDir is the shared location for skills when multiple agents are detected. - CanonicalSkillsDir = ".databricks/agent-skills" + CanonicalSkillsDir = ".databricks/aitools/skills" + + // legacySkillsDir is the old canonical location, checked for backward compatibility. + legacySkillsDir = ".databricks/agent-skills" ) // HasDatabricksSkillsInstalled checks if Databricks skills are installed in the canonical location. -// Returns true if no agents are detected (nothing to recommend) or if skills exist in ~/.databricks/agent-skills/. -// Only the canonical location is checked so that skills installed by other tools are not mistaken for a proper installation. +// Returns true if no agents are detected (nothing to recommend) or if skills exist in +// ~/.databricks/aitools/skills/ or the legacy ~/.databricks/agent-skills/. func HasDatabricksSkillsInstalled(ctx context.Context) bool { installed := DetectInstalled(ctx) if len(installed) == 0 { @@ -30,7 +33,8 @@ func HasDatabricksSkillsInstalled(ctx context.Context) bool { if err != nil { return false } - return hasDatabricksSkillsIn(filepath.Join(homeDir, CanonicalSkillsDir)) + return hasDatabricksSkillsIn(filepath.Join(homeDir, CanonicalSkillsDir)) || + hasDatabricksSkillsIn(filepath.Join(homeDir, legacySkillsDir)) } // hasDatabricksSkillsIn checks if dir contains a subdirectory starting with "databricks". diff --git a/experimental/aitools/lib/agents/skills_test.go b/experimental/aitools/lib/agents/skills_test.go index 15baba9c8f..09192d721c 100644 --- a/experimental/aitools/lib/agents/skills_test.go +++ b/experimental/aitools/lib/agents/skills_test.go @@ -134,3 +134,25 @@ func TestHasDatabricksSkillsInstalledDatabricksAppsCanonical(t *testing.T) { assert.True(t, HasDatabricksSkillsInstalled(t.Context())) } + +func TestHasDatabricksSkillsInstalledLegacyPath(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + // Skills only in the legacy location should still be detected. + require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, legacySkillsDir, "databricks"), 0o755)) + + agentDir := filepath.Join(tmpHome, ".claude") + require.NoError(t, os.MkdirAll(agentDir, 0o755)) + + origRegistry := Registry + Registry = []Agent{ + { + Name: "test-agent", + DisplayName: "Test Agent", + ConfigDir: func(_ context.Context) (string, error) { return agentDir, nil }, + }, + } + defer func() { Registry = origRegistry }() + + assert.True(t, HasDatabricksSkillsInstalled(t.Context())) +} diff --git a/experimental/aitools/lib/installer/installer.go b/experimental/aitools/lib/installer/installer.go index 084e2dc973..b9f2d7b899 100644 --- a/experimental/aitools/lib/installer/installer.go +++ b/experimental/aitools/lib/installer/installer.go @@ -2,7 +2,6 @@ package installer import ( "context" - "encoding/json" "fmt" "io" "net/http" @@ -13,7 +12,6 @@ import ( "github.com/databricks/cli/experimental/aitools/lib/agents" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/env" - "github.com/databricks/cli/libs/log" "github.com/fatih/color" ) @@ -40,39 +38,20 @@ 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"` + 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"` } // FetchManifest fetches the skills manifest from the skills repo. +// This is a convenience wrapper that uses the default GitHubManifestSource. func FetchManifest(ctx context.Context) (*Manifest, error) { + src := &GitHubManifestSource{} ref := getSkillsRef(ctx) - log.Infof(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) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) - } - - client := &http.Client{Timeout: 30 * time.Second} - resp, err := client.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to fetch manifest: %w", err) - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("failed to fetch manifest: HTTP %d", resp.StatusCode) - } - - var manifest Manifest - if err := json.NewDecoder(resp.Body).Decode(&manifest); err != nil { - return nil, fmt.Errorf("failed to parse manifest: %w", err) - } - - return &manifest, nil + return src.FetchManifest(ctx, ref) } func fetchSkillFile(ctx context.Context, skillName, filePath string) ([]byte, error) { diff --git a/experimental/aitools/lib/installer/source.go b/experimental/aitools/lib/installer/source.go new file mode 100644 index 0000000000..c32eb721d8 --- /dev/null +++ b/experimental/aitools/lib/installer/source.go @@ -0,0 +1,108 @@ +package installer + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "time" + + "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" +) + +// ManifestSource abstracts how the skills manifest and release info are fetched. +type ManifestSource interface { + // FetchManifest fetches the skills manifest at the given ref. + FetchManifest(ctx context.Context, ref string) (*Manifest, error) + + // FetchLatestRelease returns the latest release tag. + // Implementations should fall back to a default ref on network errors rather + // than returning an error. The error return exists for cases where fallback is + // not possible (e.g., mock implementations in tests that want to simulate hard + // failures). + FetchLatestRelease(ctx context.Context) (string, error) +} + +// GitHubManifestSource fetches manifests and release info from GitHub. +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.Infof(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) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + client := &http.Client{Timeout: 30 * time.Second} + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to fetch manifest: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("failed to fetch manifest: HTTP %d", resp.StatusCode) + } + + var manifest Manifest + if err := json.NewDecoder(resp.Body).Decode(&manifest); err != nil { + return nil, fmt.Errorf("failed to parse manifest: %w", err) + } + + return &manifest, nil +} + +// FetchLatestRelease returns the latest release tag from GitHub. +// If DATABRICKS_SKILLS_REF is set, it is returned immediately. +// On any error (network, non-200, parse), falls back to defaultSkillsRepoRef. +// +// The DATABRICKS_SKILLS_REF check is intentionally duplicated in getSkillsRef() +// because callers may use either the ManifestSource interface directly or the +// convenience FetchManifest wrapper. +func (s *GitHubManifestSource) FetchLatestRelease(ctx context.Context) (string, error) { + if ref := env.Get(ctx, "DATABRICKS_SKILLS_REF"); ref != "" { + return ref, nil + } + + url := fmt.Sprintf("https://api.github.com/repos/%s/%s/releases/latest", + skillsRepoOwner, skillsRepoName) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + log.Debugf(ctx, "Failed to create release request, falling back to %s: %v", defaultSkillsRepoRef, err) + return defaultSkillsRepoRef, nil + } + + client := &http.Client{Timeout: 15 * time.Second} + resp, err := client.Do(req) + if err != nil { + log.Debugf(ctx, "Failed to fetch latest release, falling back to %s: %v", defaultSkillsRepoRef, err) + return defaultSkillsRepoRef, nil + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + log.Debugf(ctx, "Latest release returned HTTP %d, falling back to %s", resp.StatusCode, defaultSkillsRepoRef) + return defaultSkillsRepoRef, nil + } + + var release struct { + TagName string `json:"tag_name"` + } + if err := json.NewDecoder(resp.Body).Decode(&release); err != nil { + log.Debugf(ctx, "Failed to parse release response, falling back to %s: %v", defaultSkillsRepoRef, err) + return defaultSkillsRepoRef, nil + } + + if release.TagName == "" { + log.Debugf(ctx, "Empty tag_name in release response, falling back to %s", defaultSkillsRepoRef) + return defaultSkillsRepoRef, nil + } + + return release.TagName, nil +} diff --git a/experimental/aitools/lib/installer/state.go b/experimental/aitools/lib/installer/state.go new file mode 100644 index 0000000000..ec17db1f08 --- /dev/null +++ b/experimental/aitools/lib/installer/state.go @@ -0,0 +1,98 @@ +package installer + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/databricks/cli/libs/env" +) + +const stateFileName = ".state.json" + +// ErrNotImplemented indicates that a feature is not yet implemented. +var ErrNotImplemented = errors.New("project scope not yet implemented") + +// InstallState records the state of all installed skills in a scope directory. +type InstallState struct { + SchemaVersion int `json:"schema_version"` + IncludeExperimental bool `json:"include_experimental,omitempty"` + Release string `json:"release"` + LastUpdated time.Time `json:"last_updated"` + Skills map[string]string `json:"skills"` + Scope string `json:"scope,omitempty"` +} + +// LoadState reads install state from the given directory. +// Returns (nil, nil) when the state file does not exist. +func LoadState(dir string) (*InstallState, error) { + data, err := os.ReadFile(filepath.Join(dir, stateFileName)) + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("failed to read state file: %w", err) + } + + var state InstallState + if err := json.Unmarshal(data, &state); err != nil { + return nil, fmt.Errorf("failed to parse state file: %w", err) + } + return &state, nil +} + +// SaveState writes install state to the given directory atomically. +// Creates the directory if it does not exist. +func SaveState(dir string, state *InstallState) error { + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("failed to create state directory: %w", err) + } + + data, err := json.MarshalIndent(state, "", " ") + if err != nil { + return fmt.Errorf("failed to marshal state: %w", err) + } + data = append(data, '\n') + + // Atomic write: write to temp file in the same directory, then rename. + tmp, err := os.CreateTemp(dir, ".state-*.tmp") + if err != nil { + return fmt.Errorf("failed to create temp file: %w", err) + } + tmpName := tmp.Name() + + if _, err := tmp.Write(data); err != nil { + tmp.Close() + os.Remove(tmpName) + return fmt.Errorf("failed to write temp file: %w", err) + } + if err := tmp.Close(); err != nil { + os.Remove(tmpName) + return fmt.Errorf("failed to close temp file: %w", err) + } + + if err := os.Rename(tmpName, filepath.Join(dir, stateFileName)); err != nil { + os.Remove(tmpName) + return fmt.Errorf("failed to rename state file: %w", err) + } + return nil +} + +// GlobalSkillsDir returns the path to the global skills directory (~/.databricks/aitools/skills/). +func GlobalSkillsDir(ctx context.Context) (string, error) { + home, err := env.UserHomeDir(ctx) + if err != nil { + return "", err + } + return filepath.Join(home, ".databricks", "aitools", "skills"), nil +} + +// ProjectSkillsDir returns the path to the project-scoped skills directory. +// Project scope is not yet implemented. +func ProjectSkillsDir(_ context.Context) (string, error) { + return "", ErrNotImplemented +} diff --git a/experimental/aitools/lib/installer/state_test.go b/experimental/aitools/lib/installer/state_test.go new file mode 100644 index 0000000000..ea459dfafe --- /dev/null +++ b/experimental/aitools/lib/installer/state_test.go @@ -0,0 +1,116 @@ +package installer + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/databricks/cli/libs/env" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoadStateNonexistentFile(t *testing.T) { + state, err := LoadState(t.TempDir()) + assert.NoError(t, err) + assert.Nil(t, state) +} + +func TestSaveAndLoadStateRoundtrip(t *testing.T) { + dir := t.TempDir() + original := &InstallState{ + SchemaVersion: 1, + Release: "v0.2.0", + LastUpdated: time.Date(2026, 3, 22, 10, 0, 0, 0, time.UTC), + Skills: map[string]string{ + "databricks": "1.0.0", + }, + } + + err := SaveState(dir, original) + require.NoError(t, err) + + loaded, err := LoadState(dir) + require.NoError(t, err) + assert.Equal(t, original, loaded) +} + +func TestSaveStateCreatesDirectory(t *testing.T) { + dir := filepath.Join(t.TempDir(), "nested", "path") + state := &InstallState{ + SchemaVersion: 1, + Release: "v0.1.0", + LastUpdated: time.Date(2026, 3, 22, 9, 0, 0, 0, time.UTC), + Skills: map[string]string{}, + } + + err := SaveState(dir, state) + require.NoError(t, err) + + // Verify file exists. + _, err = os.Stat(filepath.Join(dir, stateFileName)) + assert.NoError(t, err) +} + +func TestSaveStateTrailingNewline(t *testing.T) { + dir := t.TempDir() + state := &InstallState{ + SchemaVersion: 1, + Release: "v0.1.0", + LastUpdated: time.Date(2026, 3, 22, 9, 0, 0, 0, time.UTC), + Skills: map[string]string{}, + } + + err := SaveState(dir, state) + require.NoError(t, err) + + data, err := os.ReadFile(filepath.Join(dir, stateFileName)) + require.NoError(t, err) + assert.Equal(t, byte('\n'), data[len(data)-1]) +} + +func TestLoadStateCorruptJSON(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, stateFileName), []byte("{bad json"), 0o644)) + + state, err := LoadState(dir) + assert.Error(t, err) + assert.Nil(t, state) + assert.Contains(t, err.Error(), "failed to parse state file") +} + +func TestGlobalSkillsDir(t *testing.T) { + ctx := env.WithUserHomeDir(t.Context(), "/fake/home") + dir, err := GlobalSkillsDir(ctx) + require.NoError(t, err) + assert.Equal(t, filepath.Join("/fake/home", ".databricks", "aitools", "skills"), dir) +} + +func TestProjectSkillsDirNotImplemented(t *testing.T) { + dir, err := ProjectSkillsDir(t.Context()) + assert.ErrorIs(t, err, ErrNotImplemented) + assert.Empty(t, dir) +} + +func TestSaveAndLoadStateWithOptionalFields(t *testing.T) { + dir := t.TempDir() + original := &InstallState{ + SchemaVersion: 1, + IncludeExperimental: true, + Release: "v0.3.0", + LastUpdated: time.Date(2026, 3, 22, 12, 30, 0, 0, time.UTC), + Skills: map[string]string{ + "databricks": "1.0.0", + "sql-tools": "0.2.0", + }, + Scope: "project", + } + + err := SaveState(dir, original) + require.NoError(t, err) + + loaded, err := LoadState(dir) + require.NoError(t, err) + assert.Equal(t, original, loaded) +}