Align InstallState with spec, clean up source.go

- Rename SkillsRef->Release, LastChecked->LastUpdated (time.Time), simplify
  Skills to map[string]string, add IncludeExperimental and Scope fields.
- Remove unused Clock/RealClock from source.go.
- Add trailing newline to SaveState JSON output.
- Improve FetchLatestRelease doc comment on error/fallback contract.
- Document intentional DATABRICKS_SKILLS_REF duplication.

Co-authored-by: Isaac

Author: simonfaltum

experimental/aitools/lib/installer/source.go

@@ -11,24 +11,16 @@ import (
 	"github.com/databricks/cli/libs/log"
 )
 
-// Clock abstracts time for testing.
-type Clock interface {
-	Now() time.Time
-}
-
-// RealClock returns the actual current time.
-type RealClock struct{}
-
-// Now returns the current time.
-func (RealClock) Now() time.Time { return time.Now() }
-
 // 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.
-	// Falls back to defaultSkillsRepoRef on any error.
+	// 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)
 }
 
@@ -68,6 +60,10 @@ func (s *GitHubManifestSource) FetchManifest(ctx context.Context, ref string) (*
 // 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

experimental/aitools/lib/installer/state.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"time"
 
 	"github.com/databricks/cli/libs/env"
 )
@@ -16,18 +17,14 @@ const stateFileName = ".state.json"
 // ErrNotImplemented indicates that a feature is not yet implemented.
 var ErrNotImplemented = errors.New("project scope not yet implemented")
 
-// InstalledSkill records the installed version and timestamp for a single skill.
-type InstalledSkill struct {
-	Version     string `json:"version"`
-	InstalledAt string `json:"installed_at"`
-}
-
 // InstallState records the state of all installed skills in a scope directory.
 type InstallState struct {
-	SchemaVersion int                       `json:"schema_version"`
-	SkillsRef     string                    `json:"skills_ref"`
-	LastChecked   string                    `json:"last_checked,omitempty"`
-	Skills        map[string]InstalledSkill `json:"skills"`
+	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.
@@ -59,6 +56,7 @@ func SaveState(dir string, state *InstallState) error {
 	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")

experimental/aitools/lib/installer/state_test.go

@@ -4,6 +4,7 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/databricks/cli/libs/env"
 	"github.com/stretchr/testify/assert"
@@ -20,13 +21,10 @@ func TestSaveAndLoadStateRoundtrip(t *testing.T) {
 	dir := t.TempDir()
 	original := &InstallState{
 		SchemaVersion: 1,
-		SkillsRef:     "v0.2.0",
-		LastChecked:   "2026-03-22T10:00:00Z",
-		Skills: map[string]InstalledSkill{
-			"databricks": {
-				Version:     "1.0.0",
-				InstalledAt: "2026-03-22T09:00:00Z",
-			},
+		Release:       "v0.2.0",
+		LastUpdated:   time.Date(2026, 3, 22, 10, 0, 0, 0, time.UTC),
+		Skills: map[string]string{
+			"databricks": "1.0.0",
 		},
 	}
 
@@ -42,8 +40,9 @@ func TestSaveStateCreatesDirectory(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "nested", "path")
 	state := &InstallState{
 		SchemaVersion: 1,
-		SkillsRef:     "v0.1.0",
-		Skills:        map[string]InstalledSkill{},
+		Release:       "v0.1.0",
+		LastUpdated:   time.Date(2026, 3, 22, 9, 0, 0, 0, time.UTC),
+		Skills:        map[string]string{},
 	}
 
 	err := SaveState(dir, state)
@@ -54,6 +53,23 @@ func TestSaveStateCreatesDirectory(t *testing.T) {
 	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))
@@ -76,3 +92,25 @@ func TestProjectSkillsDirNotImplemented(t *testing.T) {
 	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)
+}