[2/5] Aitools: install writes state, interactive agent selection, idempotent install#4811
Open
simonfaltum wants to merge 5 commits intosimonfaltum/aitools-pr1-statefrom
Open
[2/5] Aitools: install writes state, interactive agent selection, idempotent install#4811simonfaltum wants to merge 5 commits intosimonfaltum/aitools-pr1-statefrom
simonfaltum wants to merge 5 commits intosimonfaltum/aitools-pr1-statefrom
Conversation
…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
4 tasks
Co-authored-by: Isaac
Co-authored-by: Isaac
Suggested reviewersBased on git history of the changed files, these people are best suited to review:
Confidence: high Eligible reviewersBased 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Stack
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 installdownloads everything, prints every file, has no state, no filtering, no agent selection.Now:
InstallSkillsForAgents(ctx, src, agents, opts)as the core install function.InstallAllSkillsbecomes a thin wrapper (signature preserved forcmd/apps/init.gocompat)..state.jsonafter successful install. Merges with existing state (doesn't overwrite).experimental: trueskills by default.min_cli_versionenforcement: skip incompatible skills with warning (hard error for single-skill install).charmbracelet/huhmulti-select. Skip prompt for 1 agent, all detected for non-interactive.Test plan
InstallAllSkillssignature preserved