diff --git a/agent/plugins/commands/update/update.go b/agent/plugins/commands/update/update.go index accd57b7..50ca6ac0 100644 --- a/agent/plugins/commands/update/update.go +++ b/agent/plugins/commands/update/update.go @@ -24,7 +24,6 @@ var askYesNo = coreutils.AskYesNo var isNonInteractive = agentcommon.IsNonInteractive const updateAllConfirmPrompt = "Update all discovered plugins under the given harness(es) to their latest version in the repository? " + - "Each install folder name is used as the repository slug (same as update --slug). " + "Matching packages will be updated, including installs that were not made with JFrog CLI." // pluginBackupDirName is the directory under the plugins parent where update backups are stored. @@ -50,45 +49,66 @@ type preUpdate struct { func RunUpdate(c *components.Context) error { all := c.GetBoolFlagValue("all") slugFlag := strings.TrimSpace(c.GetStringFlagValue("slug")) + if err := validateUpdateArgs(c, all, slugFlag); err != nil { + return err + } + + opts, err := newUpdate(c) + if err != nil { + return err + } + if all { + return runUpdateAllMode(opts) + } + return runSingleSlugUpdate(c, opts, slugFlag) +} + +func validateUpdateArgs(c *components.Context, all bool, slugFlag string) error { if !all && slugFlag == "" { if c.GetNumberOfArgs() > 0 { return fmt.Errorf("unexpected positional argument(s); use --slug to specify the plugin") } return fmt.Errorf("usage: jf agent plugins update --slug (--harness [--global] [--project-dir ] | --path ) [--repo ] [--version ] [--dry-run] [--force] [--format ]\n jf agent plugins update --all --harness [--global] [--project-dir ] [--repo ] [--dry-run] [--force] [--format ]") } - if all { - if slugFlag != "" { - return fmt.Errorf("--all cannot be combined with --slug; it updates every installed plugin for the given --harness list") - } - if c.GetNumberOfArgs() > 0 { - return fmt.Errorf("unexpected positional argument(s); use --slug or --all") - } - if strings.TrimSpace(c.GetStringFlagValue("version")) != "" { - return fmt.Errorf("--all cannot be combined with --version; it always updates to the latest version") - } - if strings.TrimSpace(c.GetStringFlagValue("path")) != "" { - return fmt.Errorf("--all cannot be combined with --path; --path targets a single install directory") - } + if !all { + return nil } - - opts, err := newUpdate(c) - if err != nil { - return err + if slugFlag != "" { + return fmt.Errorf("--all cannot be combined with --slug; it updates every installed plugin for the given --harness list") + } + if c.GetNumberOfArgs() > 0 { + return fmt.Errorf("unexpected positional argument(s); use --slug or --all") } - if all && opts.flags.AbsoluteInstallBaseDir != "" { + if strings.TrimSpace(c.GetStringFlagValue("version")) != "" { + return fmt.Errorf("--all cannot be combined with --version; it always updates to the latest version") + } + if strings.TrimSpace(c.GetStringFlagValue("path")) != "" { + return fmt.Errorf("--all cannot be combined with --path; --path targets a single install directory") + } + return nil +} + +func validateUpdateAllTargets(flags agentcommon.InstallFlagsResult) error { + if flags.AbsoluteInstallBaseDir != "" { return fmt.Errorf("--all requires --harness; --path is not supported") } - if all && len(opts.flags.Specs) == 0 { + if len(flags.Specs) == 0 { return fmt.Errorf("--all requires --harness ") } + return nil +} - if all { - if err := confirmUpdateAll(opts); err != nil { - return err - } - return runUpdateAll(opts) +func runUpdateAllMode(opts update) error { + if err := validateUpdateAllTargets(opts.flags); err != nil { + return err + } + if err := confirmUpdateAll(opts); err != nil { + return err } + return runUpdateAll(opts) +} +func runSingleSlugUpdate(c *components.Context, opts update, slugFlag string) error { if c.GetNumberOfArgs() > 0 { return fmt.Errorf("unexpected positional argument(s); use --slug to specify the plugin") } diff --git a/agent/skills/cli/cli.go b/agent/skills/cli/cli.go index 2b034da9..c95ceb21 100644 --- a/agent/skills/cli/cli.go +++ b/agent/skills/cli/cli.go @@ -38,7 +38,6 @@ func GetSubCommands() []components.Command { Name: "update", Flags: flagkit.GetCommandFlags(flagkit.SkillsUpdate), Description: "Update an installed skill.", - Arguments: getUpdateArguments(), Action: update.RunUpdate, }, { @@ -85,15 +84,6 @@ func getInstallArguments() []components.Argument { } } -func getUpdateArguments() []components.Argument { - return []components.Argument{ - { - Name: "slug", - Description: "Skill slug to update.", - }, - } -} - func getDeleteArguments() []components.Argument { return []components.Argument{ { diff --git a/agent/skills/cli/cli_test.go b/agent/skills/cli/cli_test.go index 0dc9a51f..024b3bb4 100644 --- a/agent/skills/cli/cli_test.go +++ b/agent/skills/cli/cli_test.go @@ -28,8 +28,9 @@ func TestGetSubCommands_DescriptionsAndArguments(t *testing.T) { assert.Equal(t, "Skill slug to install.", installCmd.Arguments[0].Description) updateCmd := byName["update"] + assert.NotNil(t, updateCmd.Action) + assert.Empty(t, updateCmd.Arguments) assert.Equal(t, "Update an installed skill.", updateCmd.Description) - assert.Equal(t, "Skill slug to update.", updateCmd.Arguments[0].Description) searchCmd := byName["search"] assert.Equal(t, "Search for skills in Artifactory.", searchCmd.Description) diff --git a/agent/skills/commands/update/update.go b/agent/skills/commands/update/update.go index 3cfbf65b..1bdbbd09 100644 --- a/agent/skills/commands/update/update.go +++ b/agent/skills/commands/update/update.go @@ -6,18 +6,37 @@ import ( "io/fs" "os" "path/filepath" + "sort" + "strings" agentcommon "github.com/jfrog/jfrog-cli-artifactory/agent/common" "github.com/jfrog/jfrog-cli-artifactory/agent/skills/commands/install" "github.com/jfrog/jfrog-cli-artifactory/agent/skills/commands/publish" "github.com/jfrog/jfrog-cli-artifactory/agent/skills/common" "github.com/jfrog/jfrog-cli-core/v2/plugins/components" + "github.com/jfrog/jfrog-cli-core/v2/utils/config" + "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" "github.com/jfrog/jfrog-client-go/utils/log" ) +// askYesNo is swappable in tests. +var askYesNo = coreutils.AskYesNo + +// isNonInteractive is swappable in tests (GitHub Actions sets CI=true). +var isNonInteractive = agentcommon.IsNonInteractive + +const updateAllConfirmPrompt = "Update all discovered skills under the given harness(es) to their latest version in the repository? " + + "Matching packages will be updated, including installs that were not made with JFrog CLI." + // skillBackupDirName is the directory under the skills parent where update backups are stored. const skillBackupDirName = ".skill-backup" +// resolveLatestSkillVersion is swappable in tests. +var resolveLatestSkillVersion = common.ResolveLatestSkillVersion + +// updateSlugAcrossTargetsFn is swappable in tests. +var updateSlugAcrossTargetsFn = updateSlugAcrossTargets + type preUpdate struct { agentTarget common.AgentTarget installedVersion string @@ -25,98 +44,356 @@ type preUpdate struct { failureReason string } +type update struct { + serverDetails *config.ServerDetails + repoKey string + flags agentcommon.InstallFlagsResult + dryRun bool + force bool + format string + quiet bool +} + // RunUpdate is the CLI action for `jf agent skills update`. func RunUpdate(c *components.Context) error { - if c.GetNumberOfArgs() < 1 { - return fmt.Errorf("usage: jf agent skills update (--harness [--global] [--project-dir ] | --path ) [--repo ] [--version ] [--dry-run] [--force] [--format ]") + all := c.GetBoolFlagValue("all") + slugFlag := strings.TrimSpace(c.GetStringFlagValue("slug")) + if err := validateUpdateArgs(c, all, slugFlag); err != nil { + return err } - slug := c.GetArgumentAt(0) - if err := agentcommon.ValidateSlug(slug); err != nil { + updateParams, err := newUpdate(c) + if err != nil { return err } + if all { + return runUpdateAllMode(updateParams) + } + return runSingleSlugUpdate(c, updateParams, slugFlag) +} - flags, err := common.ValidateInstallFlags(c) - if err != nil { +// validateUpdateArgs checks --slug vs --all and rejects incompatible flag combinations. +func validateUpdateArgs(c *components.Context, all bool, slugFlag string) error { + if !all && slugFlag == "" { + if c.GetNumberOfArgs() > 0 { + return fmt.Errorf("unexpected positional argument(s); use --slug to specify the skill") + } + return fmt.Errorf("usage: jf agent skills update --slug (--harness [--global] [--project-dir ] | --path ) [--repo ] [--version ] [--dry-run] [--force] [--format ]\n jf agent skills update --all --harness [--global] [--project-dir ] [--repo ] [--dry-run] [--force] [--format ]") + } + if !all { + return nil + } + if slugFlag != "" { + return fmt.Errorf("--all cannot be combined with --slug; it updates every installed skill for the given --harness list") + } + if c.GetNumberOfArgs() > 0 { + return fmt.Errorf("unexpected positional argument(s); use --slug or --all") + } + if strings.TrimSpace(c.GetStringFlagValue("version")) != "" { + return fmt.Errorf("--all cannot be combined with --version; it always updates to the latest version") + } + if strings.TrimSpace(c.GetStringFlagValue("path")) != "" { + return fmt.Errorf("--all cannot be combined with --path; --path targets a single install directory") + } + return nil +} + +// validateUpdateAllTargets ensures --all was given with --harness and not --path. +func validateUpdateAllTargets(flags agentcommon.InstallFlagsResult) error { + if flags.AbsoluteInstallBaseDir != "" { + return fmt.Errorf("--all requires --harness; --path is not supported") + } + if len(flags.Specs) == 0 { + return fmt.Errorf("--all requires --harness ") + } + return nil +} + +// runUpdateAllMode confirms with the user (when interactive) and runs update --all. +func runUpdateAllMode(updateParams update) error { + if err := validateUpdateAllTargets(updateParams.flags); err != nil { return err } + if err := confirmUpdateAll(updateParams); err != nil { + return err + } + return runUpdateAll(updateParams) +} +// runSingleSlugUpdate validates --slug and updates one skill across resolved targets. +func runSingleSlugUpdate(c *components.Context, updateParams update, slugFlag string) error { + if c.GetNumberOfArgs() > 0 { + return fmt.Errorf("unexpected positional argument(s); use --slug to specify the skill") + } + if err := agentcommon.ValidateSlug(slugFlag); err != nil { + return err + } + requestedVersion := strings.TrimSpace(c.GetStringFlagValue("version")) + return runUpdateOnSlug(updateParams, slugFlag, requestedVersion) +} + +// newUpdate parses flags, server config, and repo into the shared update state. +func newUpdate(c *components.Context) (update, error) { + flags, err := common.ValidateInstallFlags(c) + if err != nil { + return update{}, err + } serverDetails, err := agentcommon.GetServerDetails(c) if err != nil { - return err + return update{}, err } quiet := agentcommon.IsQuiet(c) repoKey, err := agentcommon.ResolveRepo(serverDetails, c.GetStringFlagValue("repo"), quiet, common.RepoOptions()) if err != nil { - return err + return update{}, err } - - requestedVersion := c.GetStringFlagValue("version") - dryRun := c.GetBoolFlagValue("dry-run") - force := c.GetBoolFlagValue("force") format := "table" if c.GetStringFlagValue("format") != "" { format = c.GetStringFlagValue("format") } + return update{ + serverDetails: serverDetails, + repoKey: repoKey, + flags: flags, + dryRun: c.GetBoolFlagValue("dry-run"), + force: c.GetBoolFlagValue("force"), + format: format, + quiet: quiet, + }, nil +} - targets, err := common.ResolveAgentTargets(slug, flags.AbsoluteInstallBaseDir, flags.Specs, flags.ProjectDirAbs, flags.IsGlobal) +// runUpdateOnSlug resolves the target version, updates all harness targets for one slug, and prints the summary. +func runUpdateOnSlug(updateParams update, slug, requestedVersion string) error { + targets, err := common.ResolveAgentTargets(slug, updateParams.flags.AbsoluteInstallBaseDir, updateParams.flags.Specs, updateParams.flags.ProjectDirAbs, updateParams.flags.IsGlobal) if err != nil { return err } - targetVersion, err := common.ResolveSkillVersion(serverDetails, repoKey, slug, requestedVersion, quiet) + targetVersion, err := common.ResolveSkillVersion(updateParams.serverDetails, updateParams.repoKey, slug, requestedVersion, updateParams.quiet) if err != nil { return err } - checks := preUpdateTargets(targets, targetVersion, force, quiet) + results, err := updateSlugAcrossTargetsFn(updateParams, slug, targetVersion, targets) + if err != nil { + return err + } + if err := agentcommon.PrintInstallSummary("Skill", slug, targetVersion, results, updateParams.format); err != nil { + return err + } + return finalError(results) +} + +type updateAllOutcome struct { + anyOK bool + anyFailed bool + firstResolveErr error + updatedSlugCount int +} + +// confirmUpdateAll prompts before update --all (skipped for --dry-run, --quiet, and CI). +func confirmUpdateAll(updateParams update) error { + if updateParams.dryRun || updateParams.quiet || isNonInteractive() { + return nil + } + if !askYesNo(updateAllConfirmPrompt, false) { + return fmt.Errorf("update --all aborted by user") + } + return nil +} + +// runUpdateAll discovers installed skills under each --harness and updates each to its latest version. +func runUpdateAll(updateParams update) error { + slugOrder, slugToTargets, err := discoverInstalledSkillTargets(updateParams.flags) + if err != nil { + return err + } + if len(slugOrder) == 0 { + log.Info("No installed skills found for the given --harness list; nothing to update.") + return nil + } + + combined, outcome := applyUpdateAllForSlugs(updateParams, slugOrder, slugToTargets) + return finalizeUpdateAll(combined, outcome, updateParams.format) +} + +// discoverInstalledSkillTargets maps each installed slug to its harness install targets. +func discoverInstalledSkillTargets(flags agentcommon.InstallFlagsResult) ([]string, map[string][]common.AgentTarget, error) { + slugToTargets := make(map[string][]common.AgentTarget) + slugOrder := make([]string, 0) + scope := common.ScopeProject + if flags.IsGlobal { + scope = common.ScopeGlobal + } + for _, spec := range flags.Specs { + installDir, err := agentcommon.ResolveAgentInstallDir(spec, flags.ProjectDirAbs, flags.IsGlobal) + if err != nil { + return nil, nil, err + } + slugs, err := discoverInstalledSkillSlugs(installDir) + if err != nil { + return nil, nil, err + } + for _, slug := range slugs { + if _, seen := slugToTargets[slug]; !seen { + slugOrder = append(slugOrder, slug) + } + slugToTargets[slug] = append(slugToTargets[slug], common.AgentTarget{ + Agent: spec, + Scope: scope, + DestinationDir: filepath.Join(installDir, slug), + }) + } + } + return slugOrder, slugToTargets, nil +} + +// discoverInstalledSkillSlugs lists skill folder names under an install directory that have SKILL.md. +func discoverInstalledSkillSlugs(installDir string) ([]string, error) { + entries, readErr := os.ReadDir(installDir) + if readErr != nil && errors.Is(readErr, os.ErrNotExist) { + return nil, nil + } + slugs := skillSlugsFromInstallDirEntries(installDir, entries) + if readErr != nil { + return slugs, fmt.Errorf("read install dir %s: %w", installDir, readErr) + } + return slugs, nil +} + +// skillSlugsFromInstallDirEntries returns sorted slugs for directories that look like installed skills. +func skillSlugsFromInstallDirEntries(installDir string, entries []os.DirEntry) []string { + slugs := make([]string, 0, len(entries)) + for _, entry := range entries { + if !entry.IsDir() { + continue + } + name := entry.Name() + if strings.HasPrefix(name, ".") { + continue + } + skillDir := filepath.Join(installDir, name) + if _, err := publish.ReadInstalledSkillVersion(skillDir); err != nil { + continue + } + slugs = append(slugs, name) + } + sort.Strings(slugs) + return slugs +} + +// applyUpdateAllForSlugs resolves latest version per slug, updates targets, and builds combined summary rows. +func applyUpdateAllForSlugs(updateParams update, slugOrder []string, slugToTargets map[string][]common.AgentTarget, +) ([]agentcommon.UpdateAllSummaryRow, updateAllOutcome) { + combined := make([]agentcommon.UpdateAllSummaryRow, 0) + var outcome updateAllOutcome + for _, slug := range slugOrder { + targetVersion, err := resolveLatestSkillVersion(updateParams.serverDetails, updateParams.repoKey, slug) + if err != nil { + if outcome.firstResolveErr == nil { + outcome.firstResolveErr = err + } + log.Warn(fmt.Sprintf("Skipping skill '%s': could not resolve latest version: %s", slug, err.Error())) + results := failedRowsForTargets(slugToTargets[slug], err.Error()) + combined = agentcommon.AppendUpdateAllSummaryRows(combined, slug, "", results) + outcome.updatedSlugCount++ + _, slugFailed := tallySummaryRows(results) + outcome.anyFailed = outcome.anyFailed || slugFailed + continue + } + results, err := updateSlugAcrossTargetsFn(updateParams, slug, targetVersion, slugToTargets[slug]) + if err != nil { + log.Warn(fmt.Sprintf("Skipping skill '%s': download failed: %s", slug, err.Error())) + results = failedRowsForTargets(slugToTargets[slug], err.Error()) + } + combined = agentcommon.AppendUpdateAllSummaryRows(combined, slug, targetVersion, results) + outcome.updatedSlugCount++ + slugOK, slugFailed := tallySummaryRows(results) + outcome.anyOK = outcome.anyOK || slugOK + outcome.anyFailed = outcome.anyFailed || slugFailed + } + return combined, outcome +} + +// failedRowsForTargets builds a failed summary row for every target with the same detail message. +func failedRowsForTargets(targets []common.AgentTarget, detail string) []agentcommon.SummaryRow { + rows := make([]agentcommon.SummaryRow, 0, len(targets)) + for _, target := range targets { + rows = append(rows, summaryRowFor(target, agentcommon.SummaryStatusFailed, detail)) + } + return rows +} + +// tallySummaryRows reports whether any per-target row succeeded or failed (skipped rows are ignored). +func tallySummaryRows(results []agentcommon.SummaryRow) (anyOK, anyFailed bool) { + for _, row := range results { + switch row.Status { + case agentcommon.SummaryStatusOK: + anyOK = true + case agentcommon.SummaryStatusFailed: + anyFailed = true + } + } + return anyOK, anyFailed +} + +// finalizeUpdateAll prints the combined --all summary and returns an error if every update failed. +func finalizeUpdateAll(combined []agentcommon.UpdateAllSummaryRow, outcome updateAllOutcome, format string) error { + if err := agentcommon.PrintUpdateAllSummary("Skill", combined, format); err != nil { + return err + } + if !outcome.anyOK && outcome.anyFailed { + return fmt.Errorf("update failed for all targets (see summary above)") + } + if !outcome.anyOK && outcome.updatedSlugCount == 0 && outcome.firstResolveErr != nil { + return outcome.firstResolveErr + } + return nil +} + +// updateSlugAcrossTargets downloads the skill once and runs the backup+copy loop per target. +func updateSlugAcrossTargets(updateParams update, slug, targetVersion string, targets []common.AgentTarget) ([]agentcommon.SummaryRow, error) { + checks := preUpdateTargets(targets, targetVersion, updateParams.force, updateParams.quiet) results, updatable := initialResultsAndUpdatable(checks, targetVersion) - if dryRun { + if updateParams.dryRun { logDryRun(slug, targetVersion, checks) - return agentcommon.PrintInstallSummary("Skill", slug, targetVersion, results, format) + return results, nil } - if len(updatable) == 0 { - if err := agentcommon.PrintInstallSummary("Skill", slug, targetVersion, results, format); err != nil { - return err - } - return finalError(results) + return results, nil } tmpDir, err := os.MkdirTemp("", "skill-update-*") if err != nil { - return fmt.Errorf("failed to create temp dir: %w", err) + return nil, fmt.Errorf("failed to create temp dir: %w", err) } defer func() { _ = os.RemoveAll(tmpDir) }() - cmd := install.NewInstallCommand(). - SetServerDetails(serverDetails). - SetRepoKey(repoKey). + installCmd := install.NewInstallCommand(). + SetServerDetails(updateParams.serverDetails). + SetRepoKey(updateParams.repoKey). SetSlug(slug). SetVersion(targetVersion). - SetQuiet(quiet). + SetQuiet(updateParams.quiet). SetSuppressSummary(true). - SetProjectDir(flags.ProjectDirAbs). - SetGlobal(flags.IsGlobal) + SetProjectDir(updateParams.flags.ProjectDirAbs). + SetGlobal(updateParams.flags.IsGlobal) - unzipDir, err := cmd.FetchAndExtractTo(tmpDir) + unzipDir, err := installCmd.FetchAndExtractTo(tmpDir) if err != nil { - return err + return nil, err } for _, preUpdateCheck := range updatable { - results = append(results, updateOneSkill(unzipDir, cmd, preUpdateCheck)) + results = append(results, updateOneSkill(unzipDir, installCmd, preUpdateCheck)) } - - if err := agentcommon.PrintInstallSummary("Skill", slug, targetVersion, results, format); err != nil { - return err - } - return finalError(results) + return results, nil } +// preUpdateTargets checks each target for install presence and whether it is already at the target version. func preUpdateTargets(targets []common.AgentTarget, targetVersion string, force, quiet bool) []preUpdate { out := make([]preUpdate, 0, len(targets)) for _, agentTarget := range targets { @@ -146,6 +423,7 @@ func preUpdateTargets(targets []common.AgentTarget, targetVersion string, force, return out } +// initialResultsAndUpdatable splits pre-checks into summary rows and targets that need a download. func initialResultsAndUpdatable(checks []preUpdate, targetVersion string) ([]agentcommon.SummaryRow, []preUpdate) { results := make([]agentcommon.SummaryRow, 0, len(checks)) updatable := make([]preUpdate, 0, len(checks)) @@ -162,6 +440,7 @@ func initialResultsAndUpdatable(checks []preUpdate, targetVersion string) ([]age return results, updatable } +// summaryRowFor builds one install-summary row for an agent target. func summaryRowFor(agentTarget common.AgentTarget, status, detail string) agentcommon.SummaryRow { return agentcommon.SummaryRow{ Agent: agentTarget.Agent.Name, @@ -172,6 +451,7 @@ func summaryRowFor(agentTarget common.AgentTarget, status, detail string) agentc } } +// logDryRun logs what would happen for each target without changing the filesystem. func logDryRun(slug, targetVersion string, checks []preUpdate) { for _, preUpdateCheck := range checks { switch { @@ -187,8 +467,7 @@ func logDryRun(slug, targetVersion string, checks []preUpdate) { } } -// updateOneSkill updates a single install target using the already-fetched tree in unzipDir: -// it renames the live install aside, copies from unzipDir, restores the backup on failure, then removes the backup on success. +// updateOneSkill backs up the current install, copies the new version, and restores on failure. func updateOneSkill(unzipDir string, installCommand *install.InstallCommand, check preUpdate) agentcommon.SummaryRow { agentTarget := check.agentTarget slugBase := filepath.Base(agentTarget.DestinationDir) @@ -229,6 +508,7 @@ func updateOneSkill(unzipDir string, installCommand *install.InstallCommand, che return summaryRowFor(agentTarget, agentcommon.SummaryStatusOK, agentcommon.SummaryDetailOKInstall) } +// finalError returns an error when every target in the summary failed. func finalError(results []agentcommon.SummaryRow) error { if len(results) == 0 { return nil @@ -241,6 +521,7 @@ func finalError(results []agentcommon.SummaryRow) error { return fmt.Errorf("update failed for all targets (see summary above)") } +// reserveUpdateBackupPath creates a unique backup directory path under .skill-backup for one slug. func reserveUpdateBackupPath(installBase, slug string) (string, error) { backupRoot := filepath.Join(installBase, skillBackupDirName) // #nosec G301 -- update backup dir under user skill tree; permissive mode matches install copy behavior for tooling access. diff --git a/agent/skills/commands/update/update_test.go b/agent/skills/commands/update/update_test.go index c5dbc922..9314d24f 100644 --- a/agent/skills/commands/update/update_test.go +++ b/agent/skills/commands/update/update_test.go @@ -10,6 +10,7 @@ import ( agentcommon "github.com/jfrog/jfrog-cli-artifactory/agent/common" "github.com/jfrog/jfrog-cli-artifactory/agent/skills/commands/install" "github.com/jfrog/jfrog-cli-artifactory/agent/skills/common" + "github.com/jfrog/jfrog-cli-core/v2/plugins/components" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -157,6 +158,127 @@ func TestUpdateOneSkill_SuccessRemovesBackup(t *testing.T) { assert.Contains(t, string(data), "2.0.0") } +func TestRunUpdate_AllRejectsSlugFlag(t *testing.T) { + ctx := newUpdateContext(t, nil, map[string]string{"harness": "cursor", "slug": "web"}, map[string]bool{"all": true}) + err := RunUpdate(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "--all cannot be combined with --slug") +} + +func TestRunUpdate_AllRejectsPositionalArg(t *testing.T) { + ctx := newUpdateContext(t, []string{"web"}, map[string]string{"harness": "cursor"}, map[string]bool{"all": true}) + err := RunUpdate(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "unexpected positional argument") +} + +func TestRunUpdate_AllRejectsVersion(t *testing.T) { + ctx := newUpdateContext(t, nil, map[string]string{"harness": "cursor", "version": "1.2.3"}, map[string]bool{"all": true}) + err := RunUpdate(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "--all cannot be combined with --version") +} + +func TestRunUpdate_AllRejectsPath(t *testing.T) { + ctx := newUpdateContext(t, nil, map[string]string{"path": t.TempDir()}, map[string]bool{"all": true}) + err := RunUpdate(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "--all cannot be combined with --path") +} + +func TestRunUpdate_RequiresSlugWithoutAll(t *testing.T) { + ctx := newUpdateContext(t, nil, map[string]string{"harness": "cursor"}, nil) + err := RunUpdate(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "usage:") +} + +func TestRunUpdate_RejectsPositionalSlug(t *testing.T) { + ctx := newUpdateContext(t, []string{"web"}, map[string]string{"harness": "cursor"}, nil) + err := RunUpdate(ctx) + require.Error(t, err) + assert.Contains(t, err.Error(), "use --slug") +} + +func TestDiscoverInstalledSkillTargets_MergesHarnesses(t *testing.T) { + projectRoot := t.TempDir() + skillsDir := ".cursor/skills" + installRoot := filepath.Join(projectRoot, skillsDir) + + dir := filepath.Join(installRoot, "shared") + require.NoError(t, os.MkdirAll(dir, agentcommon.InstallDirMode)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("---\nname: shared\nversion: 1.0.0\n---\n"), agentcommon.DefaultFileMode)) + + flags := agentcommon.InstallFlagsResult{ + Specs: []common.AgentSpec{ + {Name: "cursor", Config: agentcommon.AgentConfig{ProjectDir: skillsDir}}, + {Name: "claude", Config: agentcommon.AgentConfig{ProjectDir: skillsDir}}, + }, + ProjectDirAbs: projectRoot, + } + + slugOrder, slugToTargets, err := discoverInstalledSkillTargets(flags) + require.NoError(t, err) + require.Equal(t, []string{"shared"}, slugOrder) + require.Len(t, slugToTargets["shared"], 2) +} + +func TestDiscoverInstalledSkillTargets_SkillMdOnly(t *testing.T) { + projectRoot := t.TempDir() + skillsDir := ".cursor/skills" + installRoot := filepath.Join(projectRoot, skillsDir) + dir := filepath.Join(installRoot, "legacy") + require.NoError(t, os.MkdirAll(dir, agentcommon.InstallDirMode)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("---\nname: legacy\nversion: 0.9.0\n---\n"), agentcommon.DefaultFileMode)) + + flags := agentcommon.InstallFlagsResult{ + Specs: []common.AgentSpec{{Name: "cursor", Config: agentcommon.AgentConfig{ProjectDir: skillsDir}}}, + ProjectDirAbs: projectRoot, + } + + slugOrder, _, err := discoverInstalledSkillTargets(flags) + require.NoError(t, err) + assert.Equal(t, []string{"legacy"}, slugOrder) +} + +func TestFinalizeUpdateAll_AllFailed(t *testing.T) { + combined := []agentcommon.UpdateAllSummaryRow{ + {Agent: "cursor", Name: "a", Status: agentcommon.SummaryStatusFailed}, + } + outcome := updateAllOutcome{anyFailed: true} + err := finalizeUpdateAll(combined, outcome, "table") + require.Error(t, err) + assert.Contains(t, err.Error(), "failed for all targets") +} + +func TestConfirmUpdateAll_AbortsWhenUserDeclines(t *testing.T) { + oldAsk := askYesNo + oldCheck := isNonInteractive + defer func() { + askYesNo = oldAsk + isNonInteractive = oldCheck + }() + isNonInteractive = func() bool { return false } + askYesNo = func(_ string, _ bool) bool { return false } + + err := confirmUpdateAll(update{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "aborted by user") +} + +func newUpdateContext(t *testing.T, args []string, stringFlags map[string]string, boolFlags map[string]bool) *components.Context { + t.Helper() + ctx := &components.Context{Arguments: args} + ctx.PrintCommandHelp = func(string) error { return nil } + for name, value := range stringFlags { + ctx.AddStringFlag(name, value) + } + for name, value := range boolFlags { + ctx.AddBoolFlag(name, value) + } + return ctx +} + func skillDir(t *testing.T, skillMD string) string { t.Helper() base := t.TempDir() diff --git a/agent/skills/common/skills_util.go b/agent/skills/common/skills_util.go index b70efe97..ce88efb1 100644 --- a/agent/skills/common/skills_util.go +++ b/agent/skills/common/skills_util.go @@ -27,3 +27,19 @@ func ResolveSkillVersion(serverDetails *config.ServerDetails, repoKey, slug, req Quiet: quiet, }) } + +// ResolveLatestSkillVersion returns the greatest semver from ListVersions. +func ResolveLatestSkillVersion(serverDetails *config.ServerDetails, repoKey, slug string) (string, error) { + versions, err := ListVersions(serverDetails, repoKey, slug) + if err != nil { + return "", fmt.Errorf("failed to list versions for skill '%s': %w", slug, err) + } + if len(versions) == 0 { + return "", fmt.Errorf("skill '%s' has no versions in repository '%s'", slug, repoKey) + } + versionStrs := make([]string, len(versions)) + for idx, skillVersion := range versions { + versionStrs[idx] = skillVersion.Version + } + return agentcommon.LatestVersion(versionStrs) +} diff --git a/cliutils/flagkit/flags.go b/cliutils/flagkit/flags.go index 17214b4b..a543f657 100644 --- a/cliutils/flagkit/flags.go +++ b/cliutils/flagkit/flags.go @@ -909,7 +909,7 @@ var commandFlags = map[string][]string{ url, user, password, accessToken, serverId, repo, version, harness, projectDir, agentGlobal, installPath, agentFormat, agentQuiet, }, SkillsUpdate: { - url, user, password, accessToken, serverId, repo, version, harness, projectDir, agentGlobal, installPath, agentFormat, agentQuiet, dryRun, agentForce, + url, user, password, accessToken, serverId, repo, version, harness, projectDir, agentGlobal, installPath, agentFormat, agentQuiet, dryRun, agentForce, agentAll, agentSlug, }, SkillsDelete: { url, user, password, accessToken, serverId, repo, version, dryRun, @@ -1229,8 +1229,8 @@ var flagsMap = map[string]components.Flag{ // Skills-specific flags installPath: components.NewStringFlag(installPath, "Base directory for a direct install or update: files go under /. Mutually exclusive with --harness, --project-dir, and --global.", components.SetMandatoryFalse()), agentForce: components.NewBoolFlag(agentForce, "Re-download and reinstall even if the skill is already at the target version.", components.WithBoolDefaultValueFalse()), - agentAll: components.NewBoolFlag(agentAll, "Update every installed plugin for the given --harness list to its latest version. Mutually exclusive with --slug, --version, and --path.", components.WithBoolDefaultValueFalse()), - agentSlug: components.NewStringFlag(agentSlug, "Slug (name) of the plugin to update. Required unless --all is set.", components.SetMandatoryFalse()), + agentAll: components.NewBoolFlag(agentAll, "Update every installed package for the given --harness list to its latest version. Mutually exclusive with --slug, --version, and --path.", components.WithBoolDefaultValueFalse()), + agentSlug: components.NewStringFlag(agentSlug, "Slug (name) of the plugin or skill to update. Required unless --all is set.", components.SetMandatoryFalse()), signingKey: components.NewStringFlag(signingKey, "Path to PGP private key for signing evidence. Overrides EVD_SIGNING_KEY_PATH env var.", components.SetMandatoryFalse()), keyAlias: components.NewStringFlag(keyAlias, "Alias for the signing key. Overrides EVD_KEY_ALIAS env var.", components.SetMandatoryFalse()), agentFormat: components.NewStringFlag(Format, "Output format: \"table\" (default) or \"json\".", components.SetMandatoryFalse()),