Skip to content

Commit 60f5f7d

Browse files
aitools: handle experimental<->stable skill transitions
When a skill flips its experimental status upstream, the manifest key changes (gains or loses the -experimental suffix). Without explicit handling, install would leave the stale variant on disk and in state, and uninstall would fail to find a skill that the user typed under its other variant name. - Install: before installing a skill, check if the alternate variant is in state and remove it (delete install dir, agent symlinks, and state entry). Logs "Replaced previous variant X with Y". - Uninstall: accept either variant when the user uninstalls by name; if both are installed, remove both (the "old version" of the same logical skill is stale by definition). New `alternateVariantKey()` helper centralizes the suffix toggle. Tests cover: install replacing stale variant, uninstall by either variant when one is installed, uninstall removing both when both are installed. Co-authored-by: Isaac
1 parent 52e9d32 commit 60f5f7d

5 files changed

Lines changed: 168 additions & 4 deletions

File tree

experimental/aitools/lib/installer/installer.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,22 @@ const (
2727
skillsRepoName = "databricks-agent-skills"
2828
stableSkillsRepoPath = "skills"
2929
experimentalRepoPath = "experimental"
30+
experimentalSuffix = "-experimental"
3031
)
3132

33+
// alternateVariantKey returns the manifest key of the same logical skill
34+
// under the opposite experimental status. For "databricks-jobs" it returns
35+
// "databricks-jobs-experimental"; for "databricks-jobs-experimental" it
36+
// returns "databricks-jobs". Used to clean up the previously-installed
37+
// variant when a skill transitions between experimental and stable
38+
// upstream.
39+
func alternateVariantKey(key string) string {
40+
if strings.HasSuffix(key, experimentalSuffix) {
41+
return strings.TrimSuffix(key, experimentalSuffix)
42+
}
43+
return key + experimentalSuffix
44+
}
45+
3246
// fetchFileFn is the function used to download individual skill files.
3347
// It is a package-level var so tests can replace it with a mock.
3448
var fetchFileFn func(ctx context.Context, ref, repoDir, skillName, filePath string) ([]byte, error) = fetchSkillFile
@@ -179,6 +193,23 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
179193

180194
for _, name := range skillNames {
181195
meta := targetSkills[name]
196+
197+
// Experimental↔stable transition: if the alternate variant of this
198+
// skill was previously installed (upstream flipped its experimental
199+
// status), remove the stale variant before installing the new one.
200+
if state != nil {
201+
alt := alternateVariantKey(name)
202+
if _, ok := state.Skills[alt]; ok {
203+
altDir := filepath.Join(baseDir, alt)
204+
removeSymlinksFromAgents(ctx, alt, altDir, scope, cwd)
205+
if err := os.RemoveAll(altDir); err != nil {
206+
log.Warnf(ctx, "Failed to remove previous variant %s: %v", altDir, err)
207+
}
208+
delete(state.Skills, alt)
209+
cmdio.LogString(ctx, fmt.Sprintf("Replaced previous variant %s with %s", alt, name))
210+
}
211+
}
212+
182213
// Idempotency: skip if same version is already installed, the canonical
183214
// dir exists, AND every requested agent already has the skill on disk.
184215
if state != nil && state.Skills[name] == meta.Version {

experimental/aitools/lib/installer/installer_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,51 @@ func TestInstallProjectScopeZeroCompatibleAgentsReturnsError(t *testing.T) {
710710
assert.Contains(t, err.Error(), "No Project Agent")
711711
}
712712

713+
func TestInstallReplacesAlternateVariant(t *testing.T) {
714+
// Setup: a skill called "databricks-jobs" is installed as stable.
715+
// Then the manifest re-categorizes it as experimental (key becomes
716+
// "databricks-jobs-experimental"). A new install with --experimental
717+
// should remove the stale stable variant and install the experimental one.
718+
tmp := setupTestHome(t)
719+
ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
720+
setupFetchMock(t)
721+
agent := testAgent(tmp)
722+
723+
stableManifest := &Manifest{
724+
Version: "1",
725+
Skills: map[string]SkillMeta{
726+
"databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}},
727+
},
728+
}
729+
require.NoError(t, InstallSkillsForAgents(
730+
ctx, &mockManifestSource{manifest: stableManifest},
731+
[]*agents.Agent{agent}, InstallOptions{},
732+
))
733+
734+
globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
735+
require.DirExists(t, filepath.Join(globalDir, "databricks-jobs"))
736+
737+
// Now flip to experimental upstream. New install run.
738+
experimentalManifest := &Manifest{
739+
Version: "1",
740+
ExperimentalSkills: map[string]SkillMeta{
741+
"databricks-jobs": {Version: "0.2.0", Files: []string{"SKILL.md"}},
742+
},
743+
}
744+
require.NoError(t, InstallSkillsForAgents(
745+
ctx, &mockManifestSource{manifest: experimentalManifest},
746+
[]*agents.Agent{agent}, InstallOptions{IncludeExperimental: true},
747+
))
748+
749+
state, err := LoadState(globalDir)
750+
require.NoError(t, err)
751+
assert.NotContains(t, state.Skills, "databricks-jobs", "stale stable variant should be removed from state")
752+
assert.Equal(t, "0.2.0", state.Skills["databricks-jobs-experimental"])
753+
assert.NoDirExists(t, filepath.Join(globalDir, "databricks-jobs"), "stale stable install dir should be gone")
754+
assert.DirExists(t, filepath.Join(globalDir, "databricks-jobs-experimental"))
755+
assert.Contains(t, stderr.String(), "Replaced previous variant databricks-jobs with databricks-jobs-experimental")
756+
}
757+
713758
func TestSupportsProjectScopeSetCorrectly(t *testing.T) {
714759
expected := map[string]bool{
715760
"claude-code": true,

experimental/aitools/lib/installer/source_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ func TestFlattenManifestCollidingNamesCoexist(t *testing.T) {
6060
assert.Equal(t, "databricks-jobs", exp.SourceName, "SourceName is the un-suffixed name for the fetch URL")
6161
}
6262

63+
func TestAlternateVariantKey(t *testing.T) {
64+
assert.Equal(t, "databricks-jobs-experimental", alternateVariantKey("databricks-jobs"))
65+
assert.Equal(t, "databricks-jobs", alternateVariantKey("databricks-jobs-experimental"))
66+
// Idempotent under double application
67+
assert.Equal(t, "databricks-jobs",
68+
alternateVariantKey(alternateVariantKey("databricks-jobs")))
69+
}
70+
6371
func TestFlattenManifestEmptyStableSkills(t *testing.T) {
6472
m := &Manifest{
6573
ExperimentalSkills: map[string]SkillMeta{

experimental/aitools/lib/installer/uninstall.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,30 @@ func UninstallSkillsOpts(ctx context.Context, opts UninstallOptions) error {
6363
var toRemove []string
6464
if len(opts.Skills) > 0 {
6565
seen := make(map[string]bool)
66-
for _, name := range opts.Skills {
66+
for _, raw := range opts.Skills {
67+
// Accept either variant: if the literal name isn't installed but
68+
// the alternate variant is, use the alternate. This makes uninstall
69+
// resilient to the experimental↔stable transition.
70+
name := raw
71+
if _, ok := state.Skills[name]; !ok {
72+
if alt := alternateVariantKey(name); state.Skills[alt] != "" {
73+
name = alt
74+
} else {
75+
return fmt.Errorf("skill %q is not installed", raw)
76+
}
77+
}
6778
if seen[name] {
6879
continue
6980
}
7081
seen[name] = true
71-
if _, ok := state.Skills[name]; !ok {
72-
return fmt.Errorf("skill %q is not installed", name)
73-
}
7482
toRemove = append(toRemove, name)
83+
84+
// If both variants are installed, remove both (the alternate is
85+
// the stale "old version" of the same logical skill).
86+
if alt := alternateVariantKey(name); state.Skills[alt] != "" && !seen[alt] {
87+
seen[alt] = true
88+
toRemove = append(toRemove, alt)
89+
}
7590
}
7691
} else {
7792
for name := range state.Skills {

experimental/aitools/lib/installer/uninstall_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,71 @@ func TestUninstallSelectiveDuplicateNamesDeduplicates(t *testing.T) {
320320
assert.Contains(t, stderr.String(), "Uninstalled 1 skill.")
321321
}
322322

323+
func TestUninstallByEitherVariantRemovesBoth(t *testing.T) {
324+
// Setup: both stable and experimental variants of databricks-jobs are
325+
// installed (the transitional state we want uninstall to clean up).
326+
tmp := setupTestHome(t)
327+
ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
328+
setupFetchMock(t)
329+
agent := testAgent(tmp)
330+
331+
manifest := &Manifest{
332+
Version: "1",
333+
Skills: map[string]SkillMeta{
334+
"databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}},
335+
},
336+
ExperimentalSkills: map[string]SkillMeta{
337+
"databricks-jobs": {Version: "0.0.1", Files: []string{"SKILL.md"}},
338+
},
339+
}
340+
require.NoError(t, InstallSkillsForAgents(
341+
ctx, &mockManifestSource{manifest: manifest},
342+
[]*agents.Agent{agent}, InstallOptions{IncludeExperimental: true},
343+
))
344+
345+
globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
346+
require.DirExists(t, filepath.Join(globalDir, "databricks-jobs"))
347+
require.DirExists(t, filepath.Join(globalDir, "databricks-jobs-experimental"))
348+
349+
// Uninstall using just the un-suffixed name; both variants should go.
350+
require.NoError(t, UninstallSkillsOpts(ctx, UninstallOptions{
351+
Skills: []string{"databricks-jobs"},
352+
}))
353+
354+
assert.NoDirExists(t, filepath.Join(globalDir, "databricks-jobs"))
355+
assert.NoDirExists(t, filepath.Join(globalDir, "databricks-jobs-experimental"))
356+
assert.Contains(t, stderr.String(), "Uninstalled 2 skills.")
357+
}
358+
359+
func TestUninstallByAlternateNameWhenOnlyOneVariantInstalled(t *testing.T) {
360+
// Setup: only the experimental variant is installed.
361+
tmp := setupTestHome(t)
362+
ctx := cmdio.MockDiscard(t.Context())
363+
setupFetchMock(t)
364+
agent := testAgent(tmp)
365+
366+
manifest := &Manifest{
367+
Version: "1",
368+
ExperimentalSkills: map[string]SkillMeta{
369+
"databricks-jobs": {Version: "0.0.1", Files: []string{"SKILL.md"}},
370+
},
371+
}
372+
require.NoError(t, InstallSkillsForAgents(
373+
ctx, &mockManifestSource{manifest: manifest},
374+
[]*agents.Agent{agent}, InstallOptions{IncludeExperimental: true},
375+
))
376+
377+
globalDir := filepath.Join(tmp, ".databricks", "aitools", "skills")
378+
require.DirExists(t, filepath.Join(globalDir, "databricks-jobs-experimental"))
379+
380+
// User types the un-suffixed name (doesn't know which variant is on
381+
// disk); uninstall should still find and remove it.
382+
require.NoError(t, UninstallSkillsOpts(ctx, UninstallOptions{
383+
Skills: []string{"databricks-jobs"},
384+
}))
385+
assert.NoDirExists(t, filepath.Join(globalDir, "databricks-jobs-experimental"))
386+
}
387+
323388
func TestUninstallSelectiveAllRemovesStateFile(t *testing.T) {
324389
tmp := setupTestHome(t)
325390
globalDir := installTestSkills(t, tmp)

0 commit comments

Comments
 (0)