Skip to content

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

Open
simonfaltum wants to merge 2 commits intomainfrom
simonfaltum/aitools-pr1-state
Open

[1/5] Aitools: state tracking, manifest source, directory rename#4810
simonfaltum wants to merge 2 commits intomainfrom
simonfaltum/aitools-pr1-state

Conversation

@simonfaltum
Copy link
Member

@simonfaltum simonfaltum commented Mar 22, 2026

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 ([2/5] Aitools: install writes state, interactive agent selection, idempotent install #4811)
  3. [3/5] Update + uninstall + version commands ([3/5] Aitools: update, uninstall, and version commands #4812)
  4. [4/5] List improvements + command restructuring + flags ([4/5] Aitools: list command, flat structure, --skills/--agents/--include-experimental 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

  • LoadState returns nil, nil for nonexistent file
  • SaveState + LoadState roundtrip preserves all fields including optional ones
  • SaveState creates nested directories, writes trailing newline
  • LoadState returns error for corrupt JSON
  • GlobalSkillsDir resolves correctly
  • ProjectSkillsDir returns ErrNotImplemented
  • HasDatabricksSkillsInstalled detects legacy path
  • All existing aitools tests still pass

…tory

Add state types (InstallState) with atomic load/save persistence, path
resolution (GlobalSkillsDir, ProjectSkillsDir stub), ManifestSource
interface with GitHub implementation, latest release discovery
(FetchLatestRelease), and Clock abstraction. Extend SkillMeta with v2
fields (Experimental, Description, MinCLIVer). Refactor FetchManifest
to delegate to GitHubManifestSource. Rename canonical skills directory
from .databricks/agent-skills to .databricks/aitools/skills with
backward-compat check for the old path.

Co-authored-by: Isaac
- 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
@github-actions
Copy link

Suggested reviewers

Based on git history of the changed files, these people are best suited to review:

  • @arsenyinfo -- recent work in experimental/aitools/lib/installer/, experimental/aitools/lib/agents/
  • @pietern -- recent work in experimental/aitools/lib/installer/, experimental/aitools/lib/agents/

Confidence: high

Eligible reviewers

Based on CODEOWNERS, these people or teams could also review:

@databricks/eng-apps-devex, @lennartkats-db

Suggestions based on git history of 6 changed files (3 scored). See CODEOWNERS for path-specific ownership rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant