Skip to content

Commit ead07cf

Browse files
[1/5] Aitools: state tracking, manifest source, directory rename (#4810)
## PR Stack This is part of the aitools feature-complete initiative. PRs should be reviewed and merged in order. 1. **[1/5] State + release discovery + directory rename** (this PR) 2. [2/5] Install writes state + interactive agent selection (#4811) 3. [3/5] Update + uninstall + version commands (#4812) 4. [4/5] List improvements + command restructuring + flags (#4813) 5. [5/5] Project scope (--project/--global) (pending) Manifest v2 PR: (pending in databricks-agent-skills) ## Why The aitools skills installer has no state tracking. There's no record of what was installed, at what version, or when. This blocks every lifecycle command (update, uninstall, version). The manifest fetching logic is also hardcoded with no abstraction for testing. ## Changes Before: Skills install to `~/.databricks/agent-skills/` with no state file. `FetchManifest` is inlined in `installer.go` with a hardcoded ref and no way to mock it in tests. Now: - `InstallState` type with `LoadState`/`SaveState` (atomic write via temp+rename) for tracking installed skills, release ref, and timestamps - `ManifestSource` interface with `GitHubManifestSource`, separating manifest fetching from install logic. Enables test mocking. - `FetchLatestRelease` to discover newest release from GitHub API. Falls back to pinned ref on network failure. - v2 fields on `SkillMeta` (`Experimental`, `Description`, `MinCLIVer`) with omitempty for backward compat with v1 manifests - Canonical directory renamed from `.databricks/agent-skills` to `.databricks/aitools/skills` - Backward-compat check in `HasDatabricksSkillsInstalled` for the old path No behavior changes to existing commands except the directory path. ## Test plan - [x] `LoadState` returns nil, nil for nonexistent file - [x] `SaveState` + `LoadState` roundtrip preserves all fields including optional ones - [x] `SaveState` creates nested directories, writes trailing newline - [x] `LoadState` returns error for corrupt JSON - [x] `GlobalSkillsDir` resolves correctly - [x] `ProjectSkillsDir` returns ErrNotImplemented - [x] `HasDatabricksSkillsInstalled` detects legacy path - [x] All existing aitools tests still pass --------- Co-authored-by: Arseny Kravchenko <arsenyinfo@users.noreply.github.com>
1 parent 5f34258 commit ead07cf

File tree

6 files changed

+303
-34
lines changed

6 files changed

+303
-34
lines changed

experimental/aitools/lib/agents/skills.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,15 @@ const (
1414
databricksSkillPrefix = "databricks"
1515

1616
// CanonicalSkillsDir is the shared location for skills when multiple agents are detected.
17-
CanonicalSkillsDir = ".databricks/agent-skills"
17+
CanonicalSkillsDir = ".databricks/aitools/skills"
18+
19+
// legacySkillsDir is the old canonical location, checked for backward compatibility.
20+
legacySkillsDir = ".databricks/agent-skills"
1821
)
1922

2023
// HasDatabricksSkillsInstalled checks if Databricks skills are installed in the canonical location.
21-
// Returns true if no agents are detected (nothing to recommend) or if skills exist in ~/.databricks/agent-skills/.
22-
// Only the canonical location is checked so that skills installed by other tools are not mistaken for a proper installation.
24+
// Returns true if no agents are detected (nothing to recommend) or if skills exist in
25+
// ~/.databricks/aitools/skills/ or the legacy ~/.databricks/agent-skills/.
2326
func HasDatabricksSkillsInstalled(ctx context.Context) bool {
2427
installed := DetectInstalled(ctx)
2528
if len(installed) == 0 {
@@ -30,7 +33,8 @@ func HasDatabricksSkillsInstalled(ctx context.Context) bool {
3033
if err != nil {
3134
return false
3235
}
33-
return hasDatabricksSkillsIn(filepath.Join(homeDir, CanonicalSkillsDir))
36+
return hasDatabricksSkillsIn(filepath.Join(homeDir, CanonicalSkillsDir)) ||
37+
hasDatabricksSkillsIn(filepath.Join(homeDir, legacySkillsDir))
3438
}
3539

3640
// hasDatabricksSkillsIn checks if dir contains a subdirectory starting with "databricks".

experimental/aitools/lib/agents/skills_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,3 +134,25 @@ func TestHasDatabricksSkillsInstalledDatabricksAppsCanonical(t *testing.T) {
134134

135135
assert.True(t, HasDatabricksSkillsInstalled(t.Context()))
136136
}
137+
138+
func TestHasDatabricksSkillsInstalledLegacyPath(t *testing.T) {
139+
tmpHome := t.TempDir()
140+
t.Setenv("HOME", tmpHome)
141+
// Skills only in the legacy location should still be detected.
142+
require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, legacySkillsDir, "databricks"), 0o755))
143+
144+
agentDir := filepath.Join(tmpHome, ".claude")
145+
require.NoError(t, os.MkdirAll(agentDir, 0o755))
146+
147+
origRegistry := Registry
148+
Registry = []Agent{
149+
{
150+
Name: "test-agent",
151+
DisplayName: "Test Agent",
152+
ConfigDir: func(_ context.Context) (string, error) { return agentDir, nil },
153+
},
154+
}
155+
defer func() { Registry = origRegistry }()
156+
157+
assert.True(t, HasDatabricksSkillsInstalled(t.Context()))
158+
}

experimental/aitools/lib/installer/installer.go

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package installer
22

33
import (
44
"context"
5-
"encoding/json"
65
"fmt"
76
"io"
87
"net/http"
@@ -13,7 +12,6 @@ import (
1312
"github.com/databricks/cli/experimental/aitools/lib/agents"
1413
"github.com/databricks/cli/libs/cmdio"
1514
"github.com/databricks/cli/libs/env"
16-
"github.com/databricks/cli/libs/log"
1715
"github.com/fatih/color"
1816
)
1917

@@ -40,39 +38,20 @@ type Manifest struct {
4038

4139
// SkillMeta describes a single skill entry in the manifest.
4240
type SkillMeta struct {
43-
Version string `json:"version"`
44-
UpdatedAt string `json:"updated_at"`
45-
Files []string `json:"files"`
41+
Version string `json:"version"`
42+
UpdatedAt string `json:"updated_at"`
43+
Files []string `json:"files"`
44+
Experimental bool `json:"experimental,omitempty"`
45+
Description string `json:"description,omitempty"`
46+
MinCLIVer string `json:"min_cli_version,omitempty"`
4647
}
4748

4849
// FetchManifest fetches the skills manifest from the skills repo.
50+
// This is a convenience wrapper that uses the default GitHubManifestSource.
4951
func FetchManifest(ctx context.Context) (*Manifest, error) {
52+
src := &GitHubManifestSource{}
5053
ref := getSkillsRef(ctx)
51-
log.Infof(ctx, "Fetching skills manifest from %s/%s@%s", skillsRepoOwner, skillsRepoName, ref)
52-
url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/manifest.json",
53-
skillsRepoOwner, skillsRepoName, ref)
54-
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
55-
if err != nil {
56-
return nil, fmt.Errorf("failed to create request: %w", err)
57-
}
58-
59-
client := &http.Client{Timeout: 30 * time.Second}
60-
resp, err := client.Do(req)
61-
if err != nil {
62-
return nil, fmt.Errorf("failed to fetch manifest: %w", err)
63-
}
64-
defer resp.Body.Close()
65-
66-
if resp.StatusCode != http.StatusOK {
67-
return nil, fmt.Errorf("failed to fetch manifest: HTTP %d", resp.StatusCode)
68-
}
69-
70-
var manifest Manifest
71-
if err := json.NewDecoder(resp.Body).Decode(&manifest); err != nil {
72-
return nil, fmt.Errorf("failed to parse manifest: %w", err)
73-
}
74-
75-
return &manifest, nil
54+
return src.FetchManifest(ctx, ref)
7655
}
7756

7857
func fetchSkillFile(ctx context.Context, skillName, filePath string) ([]byte, error) {
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package installer
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"fmt"
7+
"net/http"
8+
"time"
9+
10+
"github.com/databricks/cli/libs/log"
11+
)
12+
13+
// ManifestSource abstracts how the skills manifest is fetched.
14+
type ManifestSource interface {
15+
// FetchManifest fetches the skills manifest at the given ref.
16+
FetchManifest(ctx context.Context, ref string) (*Manifest, error)
17+
}
18+
19+
// GitHubManifestSource fetches manifests from GitHub.
20+
type GitHubManifestSource struct{}
21+
22+
// FetchManifest fetches the skills manifest from GitHub at the given ref.
23+
func (s *GitHubManifestSource) FetchManifest(ctx context.Context, ref string) (*Manifest, error) {
24+
log.Infof(ctx, "Fetching skills manifest from %s/%s@%s", skillsRepoOwner, skillsRepoName, ref)
25+
url := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s/manifest.json",
26+
skillsRepoOwner, skillsRepoName, ref)
27+
28+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
29+
if err != nil {
30+
return nil, fmt.Errorf("failed to create request: %w", err)
31+
}
32+
33+
client := &http.Client{Timeout: 30 * time.Second}
34+
resp, err := client.Do(req)
35+
if err != nil {
36+
return nil, fmt.Errorf("failed to fetch manifest: %w", err)
37+
}
38+
defer resp.Body.Close()
39+
40+
if resp.StatusCode != http.StatusOK {
41+
return nil, fmt.Errorf("failed to fetch manifest: HTTP %d", resp.StatusCode)
42+
}
43+
44+
var manifest Manifest
45+
if err := json.NewDecoder(resp.Body).Decode(&manifest); err != nil {
46+
return nil, fmt.Errorf("failed to parse manifest: %w", err)
47+
}
48+
49+
return &manifest, nil
50+
}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
package installer
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"errors"
7+
"fmt"
8+
"os"
9+
"path/filepath"
10+
"time"
11+
12+
"github.com/databricks/cli/libs/env"
13+
)
14+
15+
const stateFileName = ".state.json"
16+
17+
// ErrNotImplemented indicates that a feature is not yet implemented.
18+
var ErrNotImplemented = errors.New("project scope not yet implemented")
19+
20+
// InstallState records the state of all installed skills in a scope directory.
21+
type InstallState struct {
22+
SchemaVersion int `json:"schema_version"`
23+
IncludeExperimental bool `json:"include_experimental,omitempty"`
24+
Release string `json:"release"`
25+
LastUpdated time.Time `json:"last_updated"`
26+
Skills map[string]string `json:"skills"`
27+
Scope string `json:"scope,omitempty"`
28+
}
29+
30+
// LoadState reads install state from the given directory.
31+
// Returns (nil, nil) when the state file does not exist.
32+
func LoadState(dir string) (*InstallState, error) {
33+
data, err := os.ReadFile(filepath.Join(dir, stateFileName))
34+
if errors.Is(err, os.ErrNotExist) {
35+
return nil, nil
36+
}
37+
if err != nil {
38+
return nil, fmt.Errorf("failed to read state file: %w", err)
39+
}
40+
41+
var state InstallState
42+
if err := json.Unmarshal(data, &state); err != nil {
43+
return nil, fmt.Errorf("failed to parse state file: %w", err)
44+
}
45+
return &state, nil
46+
}
47+
48+
// SaveState writes install state to the given directory atomically.
49+
// Creates the directory if it does not exist.
50+
func SaveState(dir string, state *InstallState) error {
51+
if err := os.MkdirAll(dir, 0o755); err != nil {
52+
return fmt.Errorf("failed to create state directory: %w", err)
53+
}
54+
55+
data, err := json.MarshalIndent(state, "", " ")
56+
if err != nil {
57+
return fmt.Errorf("failed to marshal state: %w", err)
58+
}
59+
data = append(data, '\n')
60+
61+
// Atomic write: write to temp file in the same directory, then rename.
62+
tmp, err := os.CreateTemp(dir, ".state-*.tmp")
63+
if err != nil {
64+
return fmt.Errorf("failed to create temp file: %w", err)
65+
}
66+
tmpName := tmp.Name()
67+
68+
if _, err := tmp.Write(data); err != nil {
69+
tmp.Close()
70+
os.Remove(tmpName)
71+
return fmt.Errorf("failed to write temp file: %w", err)
72+
}
73+
if err := tmp.Close(); err != nil {
74+
os.Remove(tmpName)
75+
return fmt.Errorf("failed to close temp file: %w", err)
76+
}
77+
78+
if err := os.Rename(tmpName, filepath.Join(dir, stateFileName)); err != nil {
79+
os.Remove(tmpName)
80+
return fmt.Errorf("failed to rename state file: %w", err)
81+
}
82+
return nil
83+
}
84+
85+
// GlobalSkillsDir returns the path to the global skills directory (~/.databricks/aitools/skills/).
86+
func GlobalSkillsDir(ctx context.Context) (string, error) {
87+
home, err := env.UserHomeDir(ctx)
88+
if err != nil {
89+
return "", err
90+
}
91+
return filepath.Join(home, ".databricks", "aitools", "skills"), nil
92+
}
93+
94+
// ProjectSkillsDir returns the path to the project-scoped skills directory.
95+
// Project scope is not yet implemented.
96+
func ProjectSkillsDir(_ context.Context) (string, error) {
97+
return "", ErrNotImplemented
98+
}
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
package installer
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
"time"
8+
9+
"github.com/databricks/cli/libs/env"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestLoadStateNonexistentFile(t *testing.T) {
15+
state, err := LoadState(t.TempDir())
16+
assert.NoError(t, err)
17+
assert.Nil(t, state)
18+
}
19+
20+
func TestSaveAndLoadStateRoundtrip(t *testing.T) {
21+
dir := t.TempDir()
22+
original := &InstallState{
23+
SchemaVersion: 1,
24+
Release: "v0.2.0",
25+
LastUpdated: time.Date(2026, 3, 22, 10, 0, 0, 0, time.UTC),
26+
Skills: map[string]string{
27+
"databricks": "1.0.0",
28+
},
29+
}
30+
31+
err := SaveState(dir, original)
32+
require.NoError(t, err)
33+
34+
loaded, err := LoadState(dir)
35+
require.NoError(t, err)
36+
assert.Equal(t, original, loaded)
37+
}
38+
39+
func TestSaveStateCreatesDirectory(t *testing.T) {
40+
dir := filepath.Join(t.TempDir(), "nested", "path")
41+
state := &InstallState{
42+
SchemaVersion: 1,
43+
Release: "v0.1.0",
44+
LastUpdated: time.Date(2026, 3, 22, 9, 0, 0, 0, time.UTC),
45+
Skills: map[string]string{},
46+
}
47+
48+
err := SaveState(dir, state)
49+
require.NoError(t, err)
50+
51+
// Verify file exists.
52+
_, err = os.Stat(filepath.Join(dir, stateFileName))
53+
assert.NoError(t, err)
54+
}
55+
56+
func TestSaveStateTrailingNewline(t *testing.T) {
57+
dir := t.TempDir()
58+
state := &InstallState{
59+
SchemaVersion: 1,
60+
Release: "v0.1.0",
61+
LastUpdated: time.Date(2026, 3, 22, 9, 0, 0, 0, time.UTC),
62+
Skills: map[string]string{},
63+
}
64+
65+
err := SaveState(dir, state)
66+
require.NoError(t, err)
67+
68+
data, err := os.ReadFile(filepath.Join(dir, stateFileName))
69+
require.NoError(t, err)
70+
assert.Equal(t, byte('\n'), data[len(data)-1])
71+
}
72+
73+
func TestLoadStateCorruptJSON(t *testing.T) {
74+
dir := t.TempDir()
75+
require.NoError(t, os.WriteFile(filepath.Join(dir, stateFileName), []byte("{bad json"), 0o644))
76+
77+
state, err := LoadState(dir)
78+
assert.Error(t, err)
79+
assert.Nil(t, state)
80+
assert.Contains(t, err.Error(), "failed to parse state file")
81+
}
82+
83+
func TestGlobalSkillsDir(t *testing.T) {
84+
ctx := env.WithUserHomeDir(t.Context(), "/fake/home")
85+
dir, err := GlobalSkillsDir(ctx)
86+
require.NoError(t, err)
87+
assert.Equal(t, filepath.Join("/fake/home", ".databricks", "aitools", "skills"), dir)
88+
}
89+
90+
func TestProjectSkillsDirNotImplemented(t *testing.T) {
91+
dir, err := ProjectSkillsDir(t.Context())
92+
assert.ErrorIs(t, err, ErrNotImplemented)
93+
assert.Empty(t, dir)
94+
}
95+
96+
func TestSaveAndLoadStateWithOptionalFields(t *testing.T) {
97+
dir := t.TempDir()
98+
original := &InstallState{
99+
SchemaVersion: 1,
100+
IncludeExperimental: true,
101+
Release: "v0.3.0",
102+
LastUpdated: time.Date(2026, 3, 22, 12, 30, 0, 0, time.UTC),
103+
Skills: map[string]string{
104+
"databricks": "1.0.0",
105+
"sql-tools": "0.2.0",
106+
},
107+
Scope: "project",
108+
}
109+
110+
err := SaveState(dir, original)
111+
require.NoError(t, err)
112+
113+
loaded, err := LoadState(dir)
114+
require.NoError(t, err)
115+
assert.Equal(t, original, loaded)
116+
}

0 commit comments

Comments
 (0)