diff --git a/internal/skills/installer/installer.go b/internal/skills/installer/installer.go index 8ae3da28f..ce5370004 100644 --- a/internal/skills/installer/installer.go +++ b/internal/skills/installer/installer.go @@ -69,6 +69,7 @@ func Install(opts *Options) (*Result, error) { skill := opts.Skills[0] if opts.OnProgress != nil { opts.OnProgress(0, 1) + defer opts.OnProgress(1, 1) } if err := installSkill(opts, skill, targetDir); err != nil { return nil, fmt.Errorf("failed to install skill %q: %w", skill.InstallName(), err) @@ -77,9 +78,6 @@ func Install(opts *Options) (*Result, error) { if err := lockfile.RecordInstall(skill.InstallName(), opts.Owner, opts.Repo, skill.Path+"/SKILL.md", skill.TreeSHA, opts.PinnedRef); err != nil { warnings = append(warnings, fmt.Sprintf("could not record install for %s: %v", skill.InstallName(), err)) } - if opts.OnProgress != nil { - opts.OnProgress(1, 1) - } return &Result{Installed: []string{skill.InstallName()}, Dir: targetDir, Warnings: warnings}, nil } diff --git a/internal/skills/installer/installer_test.go b/internal/skills/installer/installer_test.go index 0637e9c19..ea9c619c2 100644 --- a/internal/skills/installer/installer_test.go +++ b/internal/skills/installer/installer_test.go @@ -447,6 +447,42 @@ func TestInstall(t *testing.T) { } } +func TestInstallSingleSkillFailureStillCompletesProgress(t *testing.T) { + homeDir := t.TempDir() + t.Setenv("HOME", homeDir) + t.Setenv("USERPROFILE", homeDir) + + destDir := t.TempDir() + reg := &httpmock.Registry{} + defer reg.Verify(t) + reg.Register( + httpmock.REST("GET", "repos/monalisa/octocat-skills/git/trees/tree-fail"), + httpmock.StatusStringResponse(500, "server error"), + ) + client := api.NewClientFromHTTP(&http.Client{Transport: reg}) + + var events []struct{ done, total int } + result, err := Install(&Options{ + Host: "github.com", + Owner: "monalisa", + Repo: "octocat-skills", + Ref: "v1.0", + SHA: "commit123", + Client: client, + Skills: []discovery.Skill{ + {Name: "code-review", Path: "skills/code-review", TreeSHA: "tree-fail"}, + }, + Dir: destDir, + OnProgress: func(done, total int) { + events = append(events, struct{ done, total int }{done: done, total: total}) + }, + }) + + require.Error(t, err) + assert.Nil(t, result) + assert.Equal(t, []struct{ done, total int }{{done: 0, total: 1}, {done: 1, total: 1}}, events) +} + func TestResolveGitRoot(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/skills/install/install.go b/pkg/cmd/skills/install/install.go index 8188c8f3f..149d682eb 100644 --- a/pkg/cmd/skills/install/install.go +++ b/pkg/cmd/skills/install/install.go @@ -907,13 +907,15 @@ func existingSkillPrompt(targetDir string, incoming discovery.Skill) string { return fmt.Sprintf("Skill %q already exists. Overwrite?", incoming.DisplayName()) } +const installProgressLabel = "Downloading skill files" + func installProgress(io *iostreams.IOStreams, total int) func(done, total int) { - if total <= 1 { + if total <= 0 { return nil } return func(done, total int) { if done == 0 { - io.StartProgressIndicator() + io.StartProgressIndicatorWithLabel(installProgressLabel) } else if done >= total { io.StopProgressIndicator() } diff --git a/pkg/cmd/skills/install/install_test.go b/pkg/cmd/skills/install/install_test.go index a4f67f1f1..84fb24b7a 100644 --- a/pkg/cmd/skills/install/install_test.go +++ b/pkg/cmd/skills/install/install_test.go @@ -1324,6 +1324,14 @@ func TestInstallRun(t *testing.T) { } } +func TestInstallProgress(t *testing.T) { + ios, _, _, _ := iostreams.Test() + + assert.Nil(t, installProgress(ios, 0)) + assert.NotNil(t, installProgress(ios, 1)) + assert.NotNil(t, installProgress(ios, 2)) +} + func TestRunLocalInstall(t *testing.T) { tests := []struct { name string