[1/5] Aitools: state tracking, manifest source, directory rename

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".

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()))
+}

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) {

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
+}

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
+}