Skip to content

[2/5] Aitools: install writes state, interactive agent selection, idempotent install#4811

Open
simonfaltum wants to merge 5 commits intosimonfaltum/aitools-pr1-statefrom
simonfaltum/aitools-pr2-install
Open

[2/5] Aitools: install writes state, interactive agent selection, idempotent install#4811
simonfaltum wants to merge 5 commits intosimonfaltum/aitools-pr1-statefrom
simonfaltum/aitools-pr2-install

Conversation

@simonfaltum
Copy link
Member

@simonfaltum simonfaltum commented Mar 22, 2026

PR Stack

  1. [1/5] State + release discovery + directory rename ([1/5] Aitools: state tracking, manifest source, directory rename #4810)
  2. [2/5] Install writes state + interactive agent selection (this PR)
  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)

Base: simonfaltum/aitools-pr1-state (PR 1)

Why

Install currently has no state tracking, no filtering, and outputs every file download to the console. Users can't tell what version they have, and there's no way to install for specific agents.

Changes

Before: skills install downloads everything, prints every file, has no state, no filtering, no agent selection.

Now:

  • InstallSkillsForAgents(ctx, src, agents, opts) as the core install function. InstallAllSkills becomes a thin wrapper (signature preserved for cmd/apps/init.go compat).
  • State written to .state.json after successful install. Merges with existing state (doesn't overwrite).
  • Idempotent: second install skips already-present skills with matching versions.
  • Experimental filtering: skip experimental: true skills by default.
  • min_cli_version enforcement: skip incompatible skills with warning (hard error for single-skill install).
  • Interactive agent selection via charmbracelet/huh multi-select. Skip prompt for 1 agent, all detected for non-interactive.
  • Legacy install detection: prints "reinstall" message when old install found without state.
  • Concise default output (2 lines). Per-file detail only at debug level.

Test plan

  • State created after install-all and install-single
  • Experimental filtering (skip by default, include with flag)
  • min_cli_version: warning for all, hard error for single
  • Idempotent: skip same version, update changed
  • Legacy detection with helpful message
  • Interactive prompt for 2+ agents, skip for 1
  • Non-interactive: all detected, no prompt
  • InstallAllSkills signature preserved
  • Concise output, per-file at debug level

…nstall

Introduces the core InstallSkillsForAgents function that handles:
- Fetching manifest via ManifestSource interface
- Filtering experimental skills and enforcing min_cli_version
- Idempotent install (skips already-installed skills at same version)
- Legacy install detection (skills on disk without state file)
- State persistence after successful install
- Concise output (two lines: installing header + summary)

Adds interactive agent selection in cmd/skills.go using huh multi-select
when multiple agents are detected in an interactive terminal.

Preserves InstallAllSkills signature (func(context.Context) error) for
the cmd/apps/init.go callback.

Co-authored-by: Isaac
… assertion

- Fix --experimental -> --include-experimental in error message
- Merge new skills into existing state instead of overwriting
- Move FetchManifest log from Info to Debug for concise output
- Handle singular/plural in Installed N skill(s) message
- Assert min_cli_version skip warning appears in test output

Co-authored-by: Isaac
@simonfaltum simonfaltum marked this pull request as ready for review March 22, 2026 22:01
@simonfaltum simonfaltum requested review from a team and lennartkats-db as code owners March 22, 2026 22:01
@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/cmd/

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 5 changed files (4 scored). See CODEOWNERS for path-specific ownership rules.

for name, meta := range candidates {
if meta.Experimental && !opts.IncludeExperimental {
if isSpecific {
return nil, fmt.Errorf("skill %q is experimental; use --include-experimental to install", name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe just experimental?


// Save state. Merge into existing state so skills from previous installs
// (e.g., experimental skills from a prior run) are preserved.
existingState, loadErr := LoadState(globalDir)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it duplicated load?

// Both skills should be fetched because the release tag changed.
// (databricks-sql has a new version, databricks-jobs has the same version
// but state was from v0.1.0 release.)
assert.Contains(t, fetchedSkills, "databricks-sql")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to assert both?

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.

2 participants