aitools: keep experimental skill names stable

Use repo_dir as the experimental signal without suffixing install keys, and track repo dirs in state so source-folder moves still refresh installed files.

Author: simonfaltum

acceptance/experimental/aitools/skills/install-specific/output.txt

@@ -6,18 +6,18 @@ Installing Databricks AI skills for Claude Code...
 Using skills version test-ref
 Installed 1 skill.
 
-=== install a specific experimental skill (note the -experimental suffix)
->>> [CLI] experimental aitools install --global --skills test-exp-experimental --experimental
+=== install a specific experimental skill
+>>> [CLI] experimental aitools install --global --skills test-exp --experimental
 Command "install" is deprecated, use "databricks aitools install" instead.
 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-experimental
+>>> [CLI] experimental aitools install --global --skills test-exp
 Command "install" is deprecated, use "databricks aitools install" instead.
 Installing Databricks AI skills for Claude Code...
 Using skills version test-ref
-Error: skill "test-exp-experimental" is experimental; use --experimental to install
+Error: skill "test-exp" is experimental; use --experimental to install
 
 Exit code: 1

acceptance/experimental/aitools/skills/install-specific/script

@@ -4,8 +4,8 @@ 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 (note the -experimental suffix)"
-trace $CLI experimental aitools install --global --skills test-exp-experimental --experimental
+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-experimental
+errcode trace $CLI experimental aitools install --global --skills test-exp

libs/aitools/installer/installer.go

@@ -28,7 +28,6 @@ const (
 	skillsRepoName       = "databricks-agent-skills"
 	stableSkillsRepoPath = "skills"
 	experimentalRepoPath = "experimental"
-	experimentalSuffix   = "-experimental"
 )
 
 func manifestHasExperimental(m *Manifest) bool {
@@ -40,41 +39,13 @@ func manifestHasExperimental(m *Manifest) bool {
 	return false
 }
 
-// alternateVariantKey maps between "foo" and "foo-experimental".
-func alternateVariantKey(key string) string {
-	if base, ok := strings.CutSuffix(key, experimentalSuffix); ok {
-		return base
-	}
-	return key + experimentalSuffix
-}
-
-// installedSkillVersion returns the recorded version for name, or for its
-// stable/experimental alternate when only that variant is installed.
-func installedSkillVersion(state *InstallState, name string) (version string, selfInstalled, alternateInstalled bool) {
-	version, selfInstalled = state.Skills[name]
-	altVersion, alternateInstalled := state.Skills[alternateVariantKey(name)]
-	if !selfInstalled && alternateInstalled {
-		version = altVersion
-	}
-	return version, selfInstalled, alternateInstalled
-}
-
-func removeAlternateVariant(ctx context.Context, state *InstallState, baseDir, name, scope, cwd string) {
-	if state == nil {
-		return
-	}
-	alt := alternateVariantKey(name)
-	if _, ok := state.Skills[alt]; !ok {
-		return
-	}
-
-	altDir := filepath.Join(baseDir, alt)
-	removeSymlinksFromAgents(ctx, alt, altDir, scope, cwd)
-	if err := os.RemoveAll(altDir); err != nil {
-		log.Warnf(ctx, "Failed to remove previous variant %s: %v", altDir, err)
+func stateRepoDir(state *InstallState, name string) string {
+	if state != nil && state.RepoDirs != nil {
+		if repoDir := state.RepoDirs[name]; repoDir != "" {
+			return repoDir
+		}
 	}
-	delete(state.Skills, alt)
-	cmdio.LogString(ctx, fmt.Sprintf("Replaced previous variant %s with %s", alt, name))
+	return stableSkillsRepoPath
 }
 
 // fetchFileFn is the function used to download individual skill files.
@@ -123,8 +94,7 @@ type SkillMeta struct {
 	RepoDir string `json:"repo_dir,omitempty"`
 
 	// SourceName is the upstream skill directory name within RepoDir.
-	// For experimental skills the manifest key is suffixed (-experimental)
-	// but SourceName is not; set during normalization, not from JSON.
+	// Set during normalization, not from JSON.
 	SourceName string `json:"-"`
 }
 
@@ -262,11 +232,9 @@ func InstallSkillsForAgents(ctx context.Context, src ManifestSource, targetAgent
 	for _, name := range skillNames {
 		meta := targetSkills[name]
 
-		removeAlternateVariant(ctx, state, baseDir, name, scope, cwd)
-
 		// 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)
@@ -285,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
@@ -296,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

libs/aitools/installer/installer_test.go

@@ -279,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-iceberg-experimental")
+	assert.NotContains(t, state.Skills, "databricks-iceberg")
 
 	assert.Contains(t, stderr.String(), "Installed 2 skills.")
 }
@@ -309,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-iceberg-experimental")
+	assert.Contains(t, state.Skills, "databricks-iceberg")
 	assert.True(t, state.IncludeExperimental)
 
 	assert.Contains(t, stderr.String(), "Installed 3 skills.")
@@ -728,11 +728,17 @@ func TestInstallProjectScopeZeroCompatibleAgentsReturnsError(t *testing.T) {
 	assert.Contains(t, err.Error(), "No Project Agent")
 }
 
-func TestInstallReplacesAlternateVariant(t *testing.T) {
+func TestInstallKeepsNameWhenRepoDirChanges(t *testing.T) {
 	tmp := setupTestHome(t)
-	ctx, stderr := cmdio.NewTestContextWithStderr(t.Context())
-	setupFetchMock(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",
@@ -747,11 +753,13 @@ func TestInstallReplacesAlternateVariant(t *testing.T) {
 
 	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.2.0", Files: []string{"SKILL.md"}, RepoDir: experimentalRepoPath},
+			"databricks-jobs": {Version: "0.1.0", Files: []string{"SKILL.md"}, RepoDir: experimentalRepoPath},
 		},
 	}
 	require.NoError(t, InstallSkillsForAgents(
@@ -761,11 +769,10 @@ func TestInstallReplacesAlternateVariant(t *testing.T) {
 
 	state, err := LoadState(globalDir)
 	require.NoError(t, err)
-	assert.NotContains(t, state.Skills, "databricks-jobs")
-	assert.Equal(t, "0.2.0", state.Skills["databricks-jobs-experimental"])
-	assert.NoDirExists(t, filepath.Join(globalDir, "databricks-jobs"))
-	assert.DirExists(t, filepath.Join(globalDir, "databricks-jobs-experimental"))
-	assert.Contains(t, stderr.String(), "Replaced previous variant databricks-jobs with databricks-jobs-experimental")
+	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) {

libs/aitools/installer/source.go

@@ -53,23 +53,16 @@ func (s *GitHubManifestSource) FetchManifest(ctx context.Context, ref string) (*
 	return &manifest, nil
 }
 
-// normalizeManifest stamps SourceName, defaults RepoDir, and suffixes
-// experimental install keys while preserving unsuffixed upstream fetch paths.
+// normalizeManifest stamps SourceName and defaults RepoDir for older manifests.
 func normalizeManifest(m *Manifest) {
 	if m.Skills == nil {
 		m.Skills = map[string]SkillMeta{}
 	}
-	out := make(map[string]SkillMeta, len(m.Skills))
 	for name, meta := range m.Skills {
 		if meta.RepoDir == "" {
 			meta.RepoDir = stableSkillsRepoPath
 		}
 		meta.SourceName = name
-		if meta.IsExperimental() {
-			out[name+experimentalSuffix] = meta
-		} else {
-			out[name] = meta
-		}
+		m.Skills[name] = meta
 	}
-	m.Skills = out
 }

libs/aitools/installer/source_test.go

@@ -6,7 +6,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestNormalizeManifestStampsSourceNameAndSuffixesExperimental(t *testing.T) {
+func TestNormalizeManifestStampsSourceNameAndRepoDir(t *testing.T) {
 	m := &Manifest{
 		Skills: map[string]SkillMeta{
 			"databricks-apps":    {Version: "0.1.0", Files: []string{"SKILL.md"}},
@@ -21,14 +21,11 @@ func TestNormalizeManifestStampsSourceNameAndSuffixesExperimental(t *testing.T)
 	assert.Equal(t, stableSkillsRepoPath, stable.RepoDir)
 	assert.Equal(t, "databricks-apps", stable.SourceName)
 
-	exp, ok := m.Skills["databricks-iceberg-experimental"]
+	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, "SourceName preserves the upstream repo dir name (no suffix)")
-
-	_, original := m.Skills["databricks-iceberg"]
-	assert.False(t, original)
+	assert.Equal(t, "databricks-iceberg", exp.SourceName)
 }
 
 func TestManifestHasExperimental(t *testing.T) {
@@ -48,13 +45,6 @@ func TestManifestHasExperimental(t *testing.T) {
 	assert.True(t, manifestHasExperimental(withExperimental))
 }
 
-func TestAlternateVariantKey(t *testing.T) {
-	assert.Equal(t, "databricks-jobs-experimental", alternateVariantKey("databricks-jobs"))
-	assert.Equal(t, "databricks-jobs", alternateVariantKey("databricks-jobs-experimental"))
-	assert.Equal(t, "databricks-jobs",
-		alternateVariantKey(alternateVariantKey("databricks-jobs")))
-}
-
 func TestNormalizeManifestOnlyExperimentalSkills(t *testing.T) {
 	m := &Manifest{
 		Skills: map[string]SkillMeta{
@@ -64,7 +54,7 @@ func TestNormalizeManifestOnlyExperimentalSkills(t *testing.T) {
 
 	normalizeManifest(m)
 
-	got, ok := m.Skills["x-experimental"]
+	got, ok := m.Skills["x"]
 	assert.True(t, ok)
 	assert.True(t, got.IsExperimental())
 	assert.Equal(t, experimentalRepoPath, got.RepoDir)

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"`
 }
 

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",
 	}
 

libs/aitools/installer/uninstall.go

@@ -65,25 +65,13 @@ func UninstallSkillsOpts(ctx context.Context, opts UninstallOptions) error {
 		seen := make(map[string]bool)
 		for _, name := range opts.Skills {
 			if _, ok := state.Skills[name]; !ok {
-				alt := alternateVariantKey(name)
-				if _, ok := state.Skills[alt]; ok {
-					name = alt
-				} else {
-					return fmt.Errorf("skill %q is not installed", name)
-				}
+				return fmt.Errorf("skill %q is not installed", name)
 			}
 			if seen[name] {
 				continue
 			}
 			seen[name] = true
 			toRemove = append(toRemove, name)
-
-			if alt := alternateVariantKey(name); !seen[alt] {
-				if _, ok := state.Skills[alt]; ok {
-					seen[alt] = true
-					toRemove = append(toRemove, alt)
-				}
-			}
 		}
 	} else {
 		for name := range state.Skills {
@@ -101,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 {